highlight-escape-sequences icon indicating copy to clipboard operation
highlight-escape-sequences copied to clipboard

highlight-escape-sequences: Apply changes to buffers without revert.

Open wigust opened this issue 7 years ago • 6 comments

  • highlight-escape-sequences.el (turn-on-hes-mode, turn-off-hes-mode): Set font-lock-add-keywords mode to nil. Remove autoload.
  • highlight-escape-sequences.el (hes-mode): Add font-lock-flush. Set init-value to nil. Remove global keyword. Set lighter to " hes".

Hello,

This patch fixes following from issue #6:

- (turn-on-hes-mode) doesn't apply to the current buffer. To turn on hes-mode you need to M-x revert-buffer. - Same with (turn-off-hes-mode). - (turn-on-hes-mode) and (turn-off-hes-mode) works only globally, not locally for buffer.

Thanks, Oleg.

wigust avatar Nov 20 '17 06:11 wigust

Forgot to replace (hes-mode) with (add-hook 'prog-mode-hook #'hes-mode) in the commentary:

;; Put this in the init file:
;;
;; (hes-mode)

wigust avatar Nov 20 '17 06:11 wigust

That changes the mode to be non-global. And will require every existing user to change their init script.

Is that really worth it? To fix a behavior most people never notice?

dgutov avatar Nov 20 '17 17:11 dgutov

That changes the mode to be non-global. And will require every existing user to change their init script.

Yes, is it an issue to change a configuration? We could save old behavior with following patch (requires revert a buffer as before current pull request):

diff --git a/highlight-escape-sequences.el b/highlight-escape-sequences.el
index 657042d..bb4fcf9 100644
--- a/highlight-escape-sequences.el
+++ b/highlight-escape-sequences.el
@@ -237,15 +237,19 @@ Currently handles:
         (font-lock-remove-keywords nil (cdr mode))))))
 
 ;;;###autoload
-(define-minor-mode hes-mode
+(define-minor-mode hes-minor-mode
   "Toggle highlighting of escape sequences."
   :init-value nil
   :lighter " hes"
-  (if hes-mode
+  (if hes-minor-mode
       (turn-on-hes-mode)
     (turn-off-hes-mode))
   (font-lock-flush))
 
+;;;###autoload
+(define-globalized-minor-mode hes-mode
+  hes-minor-mode turn-on-hes-mode)
+
 (provide 'highlight-escape-sequences)
 
 ;;; highlight-escape-sequences.el ends here

WDYT?

wigust avatar Nov 20 '17 19:11 wigust

Yes, is it an issue to change a configuration? We could save old behavior with following patch (requires revert a buffer as before current pull request):

Revert only for global, minor works with out revert.

Is that really worth it? To fix a behavior most people never notice?

Reverting buffers is bad. It disables manually enabled minor mods via M-x.

wigust avatar Nov 20 '17 20:11 wigust

We could save old behavior with following patch

Yes. Though the result will require Emacs to run more code, for this globalized mode. Also, hes-minor-mode should be called hes-local-mode (both global and local modes are minor).

Reverting buffers is bad. It disables manually enabled minor mods via M-x.

Yeah, but how often does this situation happen? Normally, the user edits their init script once, and forgets all about this.

dgutov avatar Nov 21 '17 11:11 dgutov

Yes. Though the result will require Emacs to run more code, for this globalized mode.

Hm, is it a problem? I guess the performance is almost the same.

Also, hes-minor-mode should be called hes-local-mode (both global and local modes are minor).

OK. If it is the last suggestion then should I prepare a pull request for merging?

Reverting buffers is bad. It disables manually enabled minor mods via M-x.

Yeah, but how often does this situation happen? Normally, the user edits their init script once, and forgets all about this.

Sorry, I don't understand how is it relevant to a requirement of reverting buffers after hes-mode.

Personally I don't enable modes like hes-mode globally, because I don't need them all the time and for all buffers. But I want still be able to enable them occasionally for some buffers. I'm sure many "users" don't enable every minor mode all the time too.

wigust avatar Nov 21 '17 16:11 wigust