search

rolandwalker / fixmee

Quickly navigate to FIXME notices in Emacs
66 stars 4 forks source link

`fixmee-mode` makes every comint-derived mode unwritable #8

Closed wasamasa closed 9 years ago

wasamasa commented 9 years ago

Repro:

I'm using Emacs 24.4 on Archlinux. What could be possibly causing this?

rolandwalker commented 9 years ago

Hi! Great report!

I can duplicate this, but only with -Q.

Obviously there is a fundamental bug of some kind that needs to be addressed, but we can also add emacs-inferior-lisp-mode to customizable variable fixmee-exclude-modes. There are some interactive modes there already, as fixmee is just not designed to be useful in interactive modes. It doesn't test for derived modes, though maybe it should.

thomasf commented 9 years ago

this is maybe a duplicate of https://github.com/rolandwalker/button-lock/issues/3

wasamasa commented 9 years ago

@thomasf Thank you, that indeed looks like the reason for this peculiar behaviour.

rolandwalker commented 9 years ago

Yes, these are related. Also note, not all comint buffers are affected. This works:

It's a fairly interesting bug, which speaks to the great soup of Emacs innards.

Analysis:

All related to font-lock, of course. button-lock.el (and fixmee.el) offload all the work to font-lock, for speed. A regular expression is matched by font-lock, and the given text properties are applied on the match.

fixmee.el — one of the text properties applied to button-lock buttons by default is 'rear-nonsticky. This property defends against the other properties on the button-text accidentally spilling over into the characters that you type (which can definitely happen). The default within Emacs is for all text properties to be rear-sticky.

font-lock – meanwhile, font-lock has no idea about state. It is constantly wiping text properties over a region and then recalculating the properties from scratch. The interface for undoing text properties is therefore not very refined: one registers any affected text properties in variable font-lock-extra-managed-props, and font-lock reserves the right to wipe those. button-lock.el registers 'rear-nonsticky as a managed property, which it must do – otherwise that property never gets cleaned up.

comint.el – meanwhile, comint.el does some fairly heroic hackery to keep input, output, and prompt distinct within a buffer. It paints up the buffer with out-of-band info in the form of text properties, and assumes that nobody else will change those properties. Here we collide. comint-output-filter sets 'rear-nonsticky explicitly. But font-lock can fire off on a timer and wipe that property implicitly. And it does.

Fixes:

The ielm-prompt-read-only setting above is not a great answer, because it only covers up the issue: 'rear-nonsticky still gets wiped. The fundamental way that comint.el works is a bit fragile, but I'm not inclined to complain about that. comint.el is a sweet hack, and I think minor modes such as fixmee.el should always yield to the major mode.

Happily, fixmee.el is not really appropriate for use in an interactive buffer. Since we have a basic architectural conflict here, there's not an elegant solution; I'll make a few different improvements:

@wasamasa thank you for breaking it down from emacs -Q. I wasn't able to duplicate in my usual setup and had wrongly thought it was a transient bug.

Edit: we also need to make font-lock-extra-managed-props into a buffer-local variable.

wasamasa commented 9 years ago

Hmm, wouldn't it be simpler to just offer a customization setting for only having FIXME-like keywords highlighted and disabling button lock setup alltogether?

rolandwalker commented 9 years ago

That is the idea of #9: a customization setting to remove features.

Any user who only wants highlighting of certain terms can and should use a one-liner from the Emacs Wiki.

This package also changes the key bindings within the text, which is why it is a sort of button. That feature is one of the reasons why it was written, and that feature is also in constant use by the author.

Of course, if you use the one-liner from the Emacs Wiki, you are also using font-lock, though by exercising fewer features, you reduce the chance of encountering conflicts. It's a matter of degree.

The analysis above still applies to almost anything that we do when authoring Emacs Lisp. Emacs does not so much provide APIs as expose internals. It is not enough to set variables and invoke functions – one must also read code and analyze how other modes are using shared facilities.

wasamasa commented 9 years ago

Thank you, I'll find out how to use that oneliner from the wiki in a more flexible manner then. Feel free to mark this as closed.

rolandwalker commented 9 years ago

Thanks, I will close.

The bug should be fixed in version 0.8.6 (depends on button-lock 1.0.2), both of which have been tagged, and should show up on MELPA shortly. They have already been uploaded to Marmalade and the Emacs wiki.