Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document (?) how to change the eldoc signature #151

Open
slotThe opened this issue Jun 29, 2022 · 10 comments · May be fixed by #187
Open

Document (?) how to change the eldoc signature #151

slotThe opened this issue Jun 29, 2022 · 10 comments · May be fixed by #187

Comments

@slotThe
Copy link

slotThe commented Jun 29, 2022

Currently, when hovering over an identifier, the first "thing" that we can grab is shown in the minibuffer area (as per the default implementation of lsp-clients-extract-signature-on-hover). While this is probably fine for a lot of languages, for Haskell this seems problematic. As soon as there is at least one type class involved, the hover info will just show the instantiation of that type class and not the signature of the function at point:

2022-06-29-110620_636x131_scrot

I find this to be a bit unintuitive. It's true that knowning the specific instantiations of type classes can be very useful at times, but if one wants to know such things, there is always lsp-ui-doc-{show,glance} or lsp-describe-thing-at-point (which also show all of them and not just the first one in what is probably an arbitrary ordering).

It's possible to override lsp-clients-extract-signature-on-hover with a more appropriate implementation, but that knowledge is somewhat buried inside e.g. the rust documentation, referring to this unmerged pr. Since I think Haskell suffers from this problem as much as Rust does, it might be worth adding it to the documentation here (or somewhere else, I'm not super familiar with how documentation is structured in this project). For example, I have the following in my lsp-mode configuration

(defun slot/lsp-get-type-signature (lang str)
  "Get LANGs type signature in STR.
Original implementation from https://github.com/emacs-lsp/lsp-mode/pull/1740."
  (let* ((start (concat "```" lang))
         (groups (--filter (s-equals? start (car it))
                           (-partition-by #'s-blank? (s-lines (s-trim str)))))
         (name-at-point (symbol-name (symbol-at-point)))
         (type-sig-group (car
                          (--filter (s-contains? name-at-point (cadr it))
                                    groups))))
    (->> (or type-sig-group (car groups))
         (-drop 1)                      ; ``` LANG
         (-drop-last 1)                 ; ```
         (-map #'s-trim)
         (s-join " "))))

(cl-defmethod lsp-clients-extract-signature-on-hover 
  (contents (_server-id (eql lsp-haskell)))
  "Display the type signature of the function under point."
  (let* ((sig (slot/lsp-get-type-signature "haskell" (plist-get contents :value))))
    (lsp--render-element (concat "```haskell\n" sig "\n```"))))

which, in the above example, produces the "expected" output of

2022-06-29-120008_635x132_scrot

As noted in emacs-lsp/lsp-mode#1740, while this implementation works (even for Rust, I might add) it's probably too hacky/brittle to include somewhere verbatim, but I think it's worth to perhaps add some documentation somewhere on how to get (imo) sane hover behaviour.

@michaelpj
Copy link
Collaborator

Ugh, this is pretty ugly. I wonder if this is something we could plausibly fix in HLS. It seems reasonable to have the doc of the actual function as the "first" bit of the hover, perhaps?

@yyoncho
Copy link
Member

yyoncho commented Jun 29, 2022

@michaelpj in fact there was, but they deprecated it although IMHO it is much more useful. Check MarkedString in the spec. The best from lsp-mode perspective is to have two items one with language = <actual language> and one with markdown.

@michaelpj
Copy link
Collaborator

I meant changing HLS, not the spec :D probably we'd rather avoid deprecated stuff even if it is better

TBH, I don't quite understand the heuristic that's being used for which bit goes in the eldoc, could someone give me a one-sentence summary? Then we can maybe make HLS produce output that does the right thing.

@slotThe
Copy link
Author

slotThe commented Jun 29, 2022

TBH, I don't quite understand the heuristic that's being used for which bit goes in the eldoc, could someone give me a one-sentence summary? Then we can maybe make HLS produce output that does the right thing.

Having the type signature as the actual first bit of the response would get us most of the way there, I think. Currently, a typical response we want to process looks like this:

```haskell
$dFoldable :: Foldable []
```

* * *

```haskell
$dContravariant :: Contravariant f
```

* * *

```haskell
$dApplicative :: Applicative f
```

* * *

```haskell
folding :: forall (f :: * -> *) s a. Foldable f => (s -> f a) -> Fold s a
```

*Defined in ‘Control.Lens.Fold’*
 *(lens-5.0.1)*
* * *

```haskell
_ :: (MultiMap k v -> [(k, v)])
-> ((k, v) -> f (k, v)) -> MultiMap k v -> f (MultiMap k v)
```
* * *

```haskell
_ :: forall (f :: * -> *) s a. Foldable f => (s -> f a) -> Fold s a
```

What we probably want to extract here is

folding :: forall (f :: * -> *) s a. Foldable f => (s -> f a) -> Fold s a

The default implementation (where contents is a response the like above) for lsp-clients-extract-signature-on-hover is

(car (s-lines (s-trim (lsp--render-element contents))))

which essentially just gets the first line (modulo the GFM markup), which would be $dFoldable :: Foldable [] here. If we pushed the type signature to the very front, we still could not quite use this (ime, it's not uncommon for the signature to span over multiple lines), but the override would be trivial: "get everything in the first Haskell GFM block".

What I did above more or less does the following:

  1. Search through all Haskell code blocks for the symbol at point; since this is the thing we want to have a signature for, this seems like a safe assumption.
  2. Take the first one if multiple matches are found—this assumes that e.g. usage examples come after the signature (which I haven't seen violated so far).
  3. In case nothing could be found that matches, just use the first thing inside some GFM block (i.e., more or less fall back to the default implementation).

@michaelpj
Copy link
Collaborator

I don't know whether HLS will ever actually return a signature across multiple lines, so reordering it might just be sufficient. It's probably better UX for the hover itself too!

@slotThe
Copy link
Author

slotThe commented Jun 29, 2022

I don't know where it happens (on the HLS side or the lsp-mode side) but we definitely receive multi-line type signatures in Emacs. For example, the function

iAmTooLong :: String -> String -> String -> String -> String -> String -> String -> String
iAmTooLong = undefined

will generate the response

```haskell
iAmTooLong :: String
-> String
-> String
-> String
-> String
-> String
-> String
-> String
```

*Defined at /home/slot/repos/haskell/sandbox/src/Main.hs:4:1*

By default, only iAmTooLong :: String is shown in the minibuffer.

@michaelpj
Copy link
Collaborator

Great example. That seems like something that could plausibly be improved in lsp-mode, some behaviour like "grab the entire contents of the first GFM block if there is one". WDYT @yyoncho ?

@michaelpj michaelpj reopened this Jun 29, 2022
@michaelpj
Copy link
Collaborator

(For some reason since yesterday Ctrl-Enter now triggers close+comment instead of comment. I'm going insane.)

@slotThe
Copy link
Author

slotThe commented Jan 26, 2024

Any updates on this? FWIW, I've been using a variation of the code posted in the OP for quite a while now, and it never broke :)

@michaelpj
Copy link
Collaborator

I'd accept a PR that changes things, but I'd prefer a generic change in lsp-mode if possible.

slotThe added a commit to slotThe/lsp-haskell that referenced this issue Aug 17, 2024
This fixes a case where a type signature would be broken up over
multiple lines, and we only display the first of these.

Fixes: emacs-lsp#151
Related: emacs-lsp/lsp-mode#4362
Related: emacs-lsp/lsp-mode#1740
@slotThe slotThe linked a pull request Aug 17, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants