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

[layers/+lang/haskell] Prolong flycheck popup duration in Haskell layer. #13803

Merged

Conversation

Martinsos
Copy link
Contributor

While recently using haskell layer, I realized that flycheck popups dissapear after 5 seconds, which was making my development really hard -> error messages in haskell are usually pretty complex and lenghty, and trying to read them in 5 seconds is in most cases impossible, so I would repeatedly have to move and return my cursor on the position of error to see the message again and again.

After some digging and help from people on gitter, I learned that by default flycheck shows popup for 5 seconds (this default is coming from flycheck-pos-tip), and that duration of popup can be modified with flycheck-pos-tip-timeout variable. I put it to 20 seconds since I believe that is fairly long, we could even put it to something longer. I am not even sure if this popup timeout is useful at all to be honest.

This is my first commit and I am not very proficient with elisp so I put it is most obvious place and I can confirm that it works, however I don't know if there is a better location to put this command, I am hoping you will tell me!

@Martinsos Martinsos changed the title Prolong flycheck popup duration in Haskell layer. [Haskell layer] Prolong flycheck popup duration in Haskell layer. Jul 30, 2020
@Martinsos Martinsos changed the title [Haskell layer] Prolong flycheck popup duration in Haskell layer. [layers/+lang/haskell] Prolong flycheck popup duration in Haskell layer. Jul 30, 2020
@duianto
Copy link
Contributor

duianto commented Aug 6, 2020

Setting (setq flycheck-pos-tip-timeout 20) like that, will probably change it in every mode.

Let's make a syntax-checking layer variable instead.

Remove that variable assignment (setq flycheck-pos-tip-timeout 20)

Add these lines:

(defvar syntax-checking-auto-hide-tooltips 5
  "Seconds until the tooltip auto hides (default 5).
If nil, keep tooltips open until the cursor is moved.")

before this syntax-checking-enable-tooltips variable declaration:

(defvar syntax-checking-enable-tooltips t

Then in the syntax-checking/init-flycheck-pos-tip function:

(defun syntax-checking/init-flycheck-pos-tip ()

after the flycheck-pos-tip-mode activation.

Add this variable assignment:

(setq flycheck-pos-tip-timeout (or syntax-checking-auto-hide-tooltips -1))

The function ends up looking like this:

(defun syntax-checking/init-flycheck-pos-tip ()
  (use-package flycheck-pos-tip
    :if syntax-checking-enable-tooltips
    :defer t
    :init
    (with-eval-after-load 'flycheck
      (flycheck-pos-tip-mode)
      (setq flycheck-pos-tip-timeout (or syntax-checking-auto-hide-tooltips -1)))))

That will make it possible to change the tooltip duration by adding the new variable to the
dotspacemacs-configuration-layers list near the top of the .spacemacs file, like this:

Keep the tooltip open until the cursor is moved:

     (syntax-checking :variables syntax-checking-auto-hide-tooltips nil)

Or set it to a specific number of seconds:

     (syntax-checking :variables syntax-checking-auto-hide-tooltips 20)

And could you add an entry in changelog.develop.

In the Syntax-checking section:
https://github.com/syl20bnr/spacemacs/blob/develop/CHANGELOG.develop#syntax-checking-1

On a new line before the following section: Swift

It could say something like this:

- Added layer variable =syntax-checking-auto-hide-tooltips= (thanks to Martin Sosic)

Copy link
Collaborator

@smile13241324 smile13241324 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check duiantos review and fix the open points. This setting is not layer specific but rather a flycheck wide setting and should therefore be part of the syntax-checking layer to be configured.

The default could be 10 seconds, but is documented in "syntax-checking" layer. With this people could adapt the popup timeout to match their habit.

Remember that this is only valid for non lsp powered modes. If lsp is used lsp-ui will take care to project the warning into the buffer without a popup, hence there is no timeout.

@duianto
Copy link
Contributor

duianto commented Aug 8, 2020

Also document the new layer variable in syntax-checking/readme.org

After the Enabling/Disabling tooltips section
https://github.com/syl20bnr/spacemacs/blob/develop/layers/+checkers/syntax-checking/README.org#enablingdisabling-tooltips

Add this or something more descriptive:

** Auto hide tooltips
By default tooltips are hidden after 5 seconds.

The auto hide time can be changed by setting the variable
=syntax-checking-auto-hide-tooltips= to a number.

Or to keep the tooltips open until the cursor is moved, set the variable to =nil=.

#+BEGIN_SRC emacs-lisp
  (setq-default dotspacemacs-configuration-layers
    '((syntax-checking :variables syntax-checking-auto-hide-tooltips nil)))
#+END_SRC

@smile13241324
Copy link
Collaborator

Any news here, @Martinsos? Do you wish to try to implement duiantos idea, or should I take over that change?

@Martinsos
Copy link
Contributor Author

Martinsos commented Aug 20, 2020 via email

@Martinsos
Copy link
Contributor Author

@duianto and @smile13241324 thank you for very detailed instructions, since I am beginner with spacemacs they helped a lot and helped me learn more about how spacemacs works.

I implemented your suggestions, with only difference being using 0 instead of -1 for flycheck-pos-tip-timeout when we want tooltips to not auto hide, since in pos-tip documentation they said any non-positive number works, and I felt 0 is more meaningful for this purpose than -1.

I believe this is it, but I do still have questions:

  1. While this enables spacemacs users to easily set the timeout of tooltips, it still does not solve the problem I originally wanted to solve, which was that when using haskell layer to write haskell code, default timeout of tooltips (which is 5 seconds) makes no sense. So while user can now easily modify that, I still think it would be great if haskell layer took care of that immediately instead of everybody having to figure out on their own that they should configure it themselves. Is there a way in haskell layer to configure syntax-checking-auto-hide-tooltips just for itself, without affecting other language layers?
  2. In general, why do we have 5 seconds as default value? Why do we even need the timeout, in practice? I can't see the purpose of it. Should be set it by default to 0 or at least some bigger value (10, 20 seconds) and users can configure it if needed? Maybe we could run some kind of poll to see if people find timeout useful?

Copy link
Collaborator

@smile13241324 smile13241324 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good so far, just the default I would change to nil so that the popups do not longer autohide

@smile13241324
Copy link
Collaborator

@Martinsos while I agree that a default timeout should be set to 0, as I would also expect a tooltip not to disappear automatically, this is still unrelated to the language used. In spacemacs all auto complete and syntax checking decisions are centralised so that these behave consistent in all languages, therefore this setting cannot be set on a layer basis.

@duianto do you see any reasons why not to set the default to "0" hence let the popup not disappear automatically?

@smile13241324
Copy link
Collaborator

Btw @Martinsos can you add your change to the changelog.develop please, we hope to use this for documenting the main changes when we finally have another release.

@duianto
Copy link
Contributor

duianto commented Aug 25, 2020

why do we have 5 seconds as default value?

I just used the default value that's defined upstream in flycheck-pos-tip:
https://github.com/flycheck/flycheck-pos-tip/blob/dc57beac0e59669926ad720c7af38b27c3a30467/flycheck-pos-tip.el#L56


do you see any reasons why not to set the default to "0" hence let the popup not disappear automatically?

There seems to be some reports requesting both behaviors:

I don't have a preference either way.

But since you mentioned that lsp mode doesn't have a timeout for the popups, then it might be more consistent if both types of tooltips (lsp and flycheck) behaved the same.

@smile13241324
Copy link
Collaborator

Yep, I have not yet noticed that, you are right yes LSP has no popup hence there is no timeout the information is projected into the right side of the screen as long as your cursor is on the warning. So lets set the default to no popup, then the people who wish to have it autohide can set it accordingly, and the others who do not want to see any popup and rather see it in the mini buffer, eldoc style, can disable tool-tips all together.

@Martinsos
Copy link
Contributor Author

@smile13241324 and @duianto sounds good, I am also in favor of not auto-hiding the tooltips by default. I added commit where I set syntax-checking-auto-hide-tooltips to nil by default.

@smile13241324 regarding changelog.develop, I already added the change there, is that it?

@duianto
Copy link
Contributor

duianto commented Aug 25, 2020

The readme needs to be updated to reflect that the tooltips aren't hidden automatically by default.

And can you squash the commits, the recommendation https://github.com/syl20bnr/spacemacs/blob/develop/CONTRIBUTING.org#ideally-for-simple-prs-most-of-them
is to limit the commits to one per PR, unless the changes are complex.

@Martinsos Martinsos force-pushed the lang-haskell-popup-duration branch from 9f653cd to 3295b78 Compare August 25, 2020 19:58
@Martinsos
Copy link
Contributor Author

@duianto ah I forgot about the README! Ok updated, and I squashed the commits! I normally squash commits when working on my code, but I do it when PR is done so I don't mess up with the comments on github because it can happen they become outdated otherwise and it becomes harder to track the conversation.

@duianto
Copy link
Contributor

duianto commented Aug 25, 2020

I'm observing the issue that's described here:
Tool-tip not disappearing flycheck/flycheck-pos-tip#6

The tooltip is displayed on top of all applications when switching to an application that's on top of Emacs.

  • Move the cursor to an error so that the tooltip appears

emacs_2020-08-25_22-40-32

  • Switch to another application (Alt + Tab)
    the tooltip remains on top of the application that one switched to

chrome_2020-08-25_22-40-33

It seems to have been fixed when it was reported in 2015.

Maybe something's changed since then.

Update

I reported it in that upstream issue: flycheck/flycheck-pos-tip#6 (comment)
it's reproducable without Spacemacs, in both NixOS and Windows.

@Martinsos
Copy link
Contributor Author

@duianto true, I can also replicate this! Never noticed it before.
I don't think we should change behavior to cover for this bug though, if somebody has this problem they can set the timeout themselves, if that helps.

@smile13241324 smile13241324 merged commit 7732fca into syl20bnr:develop Aug 26, 2020
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 this pull request may close these issues.

3 participants