flycheck / flycheck-inline

Display Flycheck errors inline
GNU General Public License v3.0
137 stars 10 forks source link

Make flycheck-inline-mode buffer-local #9

Closed fmdkdd closed 5 years ago

fmdkdd commented 5 years ago

Close GH-8.

Not sure this is the proper way to do it, but it seems to work.

We probably want to make flycheck-display-errors-function buffer-local directly in flycheck.el, as the setq-local call will make that variable buffer-local as a side effect of enabling flycheck-inline mode.

Or maybe there is a way to make that variable global again when we disable the mode?

fmdkdd commented 5 years ago

One downside of this PR is that we are changing the behavior of flycheck-inline-mode to be local instead of global.

cpitclaudel commented 5 years ago

You could do that, or instead you could call the local function flycheck-inline-local-mode.

edkolev commented 5 years ago

I don't see the point of flycheck-inline providing a global mode at all - it doesn't make sense to have it in the plethora of buffers in which flycheck isn't itself on.

To keep backwards compatibility, I would suggest: define flycheck-inline-local-mode as a local mode, then define a global flycheck-inline-mode which only adds/removes a hook (add-hook 'flyckech-mode 'flycheck-inline-local-mode).

fmdkdd commented 5 years ago

I don't see the point of flycheck-inline providing a global mode at all - it doesn't make sense to have it in the plethora of buffers in which flycheck isn't itself on.

That sounds better indeed. It's unclear what the best practice is in this situation. I based the code off flycheck-pos-tip, for which the same argument could be made. @cpitclaudel WDYT?

cpitclaudel commented 5 years ago

I don't see the point of flycheck-inline providing a global mode at all - it doesn't make sense to have it in the plethora of buffers in which flycheck isn't itself on.

I'm not entirely convinced by this argument: flycheck-inline can both (1) be a global mode and (2) not be turned on when flycheck-mode isn't.

Ideally, turning on the global mode should turn on the behavior in all buffer where flycheck is already active, too.

fmdkdd commented 5 years ago

Ok, I've made a small change to only turn on the local flycheck-inline-mode in flycheck-mode buffers. Users are free to not use the global-flycheck-inline-mode, and just turn on flycheck-inline-mode using a hook to flycheck-mode as suggested by @edkolev.

I've added a commit to fix #7 as well.