Use echo area in nongraphical sessions

[cpitclaudel]

Here's an tentative implementation for #11. It adds two fallback functions -tty-show and -tty-hide that are used on TTYs instead of the normal show and hide.

I wonder whether adding these two fallback functions is the best though. Since the show and hide functions are already customizable, we could instead just make the defaults for these two a bit smarter (this would also be a bit more backwards compatible).

Also worthy of discussion: how much trouble do we want to go through to make tty tooltips disappear after the right amount of time? We could save the previous message (in the style of with-temp-message) and restore it from a timer, but is that complexity warranted in the tty case?

Side note: the flycheck-pos-tip-timeout setting is a bit surprising, as it's not enforced independently of the value of flycheck-pos-tip-show-function (the default -show function respects it, of course, but we could enforce it by calling -hide in a timer after flycheck-pos-tip-timeout).

@lunaryorn, opinions? :)

[swsnr]

@cpitclaudel I think we should keep the fallback as simple as possible. I presume anyone who adds this extension has a preference for GUI Emacs anyway; I guess we can live with a less perfect fallback.

Specifically, I'd not bother to restore the old message (the standard error display doesn't either) nor enforce the timeout for the fallback function. If a user wants that they can write their own more sophisticated fallback.

We need a separate fallback function, however; a global default won't cater for the type of the individual frame, which is inherently a frame-local state. Just consider a (frame-less) Emacs daemon that gets a GUI frame and a TTY frame attached: The former will be able to and should use pos-tip, while the latter needs the fallback. We have to make the choice individually for the current frame.

As for flycheck-pos-tip-timeout, I think we should just document (i.e. in the docstring) that the variable only has effect with the default show function, and only for the non-fallback case.

[swsnr]

"environment" -> "frame". That's the proper Emacs terminology I think.

[cpitclaudel]

I've implemented the changes you suggested, thanks for the review :)

We need a separate fallback function, however; a global default won't cater for the type of the individual frame, which is inherently a frame-local state. Just consider a (frame-less) Emacs daemon that gets a GUI frame and a TTY frame attached: The former will be able to and should use pos-tip, while the latter needs the fallback. We have to make the choice individually for the current frame.

Indeed. My thought was just that instead of providing two configurable show functions (one for GUI and one for TTY, defaulting respectively to pos-tip and message), we could provide only one, the default implementation of which would check (display-graphic-p).

That is, instead of putting

(funcall (if (display-graphic-p)
                   flycheck-pos-tip-show-function
                 flycheck-pos-tip-tty-show-function)
               messages)

in flycheck-pos-tip-error-messages, we could put the check in flycheck-pos-tip-show-function.

I don't think it matters too much, however.

[swsnr]

ignore is the nicer default here, I think… it doesn't make any difference in behaviour, but it just reads nicer in my opinion.

[swsnr]

@cpitclaudel Ah, so you mean we wouldn't have a separate option for the TTY fallback, but just a single function that dispatches accordingly, right?

That's a nice idea, too, I didn't think of that. I guess that'd be much simpler.

[cpitclaudel]

@cpitclaudel Ah, so you mean we wouldn't have a separate option for the TTY fallback, but just a single function that dispatches accordingly, right?

Yes, exactly. I'll update the patch accordingly :)

[cpitclaudel]

Patch updated :) I'll squash everything before merging, but I thought it best to keep both commits for now.

[swsnr]

I'm not sure whether that actually works. As far as I can tell, flycheck-display-error-messages wants a list of error objects as argument. We might need to change flycheck-pos-tip-function so that it takes a list of errors instead strings…

[cpitclaudel]

Indeed, you're entirely right; I wasn't loading the right file when I tested it. I've pushed a new patch.

We might need to change flycheck-pos-tip-function so that it takes a list of errors instead strings…

We could do this, but it'd break backwards compatibility (though I don't imagine may people actually customize that function). OOTH, duplicating the bit of functionality of flycheck-display-error-messages that we're interested in creates very very minor code duplication, so maybe it's acceptable.

[swsnr]

@cpitclaudel Merged, thank you ☺️

I've refactored things a bit, though. I removed the customisation options for the show/hide functions. It was a bad idea of mine to introduce these in the first place; if anyone wants to use a different library to show popups they are better of with their own library and minor mode anyway, because this one has many assumptions about the behaviour of pos-tip and X tooltips baked in that'd sooner or later conflict with other popup libraries anyway.

However, I introduced an option to customise the fallback function. I think that this is actually needed, particularly users which use the error list a lot would probably rather use flycheck-display-error-messages-unless-error-list as fallback instead.

[cpitclaudel]

Awesome, that looks great!