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

Some modes require a second mode activation #26

Closed
blaenk opened this issue Jun 16, 2018 · 31 comments
Closed

Some modes require a second mode activation #26

blaenk opened this issue Jun 16, 2018 · 31 comments
Assignees
Labels

Comments

@blaenk
Copy link

blaenk commented Jun 16, 2018

I'm not quite sure how to describe this, but for example with sql-mode buffers, when I first open the buffer, TODOs in comments aren't highlighted. If I disable the mode, then enable it again, then the keywords are highlighted. I checked and sql-mode derives prog-mode.

@dschrempf
Copy link

I am having the same problem with latex-mode which is not derived from prog-mode but I have hl-todo explicitely enabled. The TODO keywords are found with, e.g., hl-todo-next (but not highlighted). Deactivation with subsequent activation enables highlighting.

@tarsius
Copy link
Owner

tarsius commented Jun 19, 2018

Yeah me too. It's quite annoying but I never got around to investigate enough to actually fix it because I suspect doing so would involve gaining a deep understanding of how font-locking works and implementing functionality which ought to already exist.

Back in the days package authors used font-lock-fontify-buffer which wasn't intended for this purpose of making new keywords take affect but by accident accomplished to do so (sometimes).

(defun font-lock-fontify-buffer (&optional interactively)
  "Fontify the current buffer the way the function `font-lock-mode' would."
  (declare
   ;; When called from Lisp, this function is a big mess.  The caller usually
   ;; expects one of the following behaviors:
   ;; - refresh the highlighting (because the font-lock-keywords have been
   ;;   changed).
   ;; - apply font-lock highlighting even if font-lock-mode is not enabled.
   ;; - reset the highlighting rules because font-lock-defaults
   ;;   has been changed (and then rehighlight everything).
   ;; Of course, this function doesn't do all of the above in all situations
   ;; (e.g. depending on whether jit-lock is in use) and it can't guess what
   ;; the caller wants.
   (interactive-only "use `font-lock-ensure' or `font-lock-flush' instead."))
  (interactive "p")
  (font-lock-set-defaults)
  (let ((font-lock-verbose (or font-lock-verbose interactively)))
    (funcall font-lock-fontify-buffer-function)))

The commit that added the above comment, emacs-mirror/emacs@6711a21, also added font-lock-ensure and font-lock-flush.

I tried using the former instead for a while because its doc-string says that (when called without any arguments) it makes sure that the whole buffer is fontified, but apparently that only means that ensures that the buffer has been fontified at some point and not that it ensures that all currently defined keywords actually are in effect.

So I started to also call font-lock-flush before font-lock-ensure, the former "declares" that the fontification is out-of-date, and the latter "ensures" that fontification is in effect, though apparently (in some cases only?) this still isn't enough to actually put the newly added keywords into effect.

(define-minor-mode hl-todo-mode
  "Highlight TODO and similar keywords in comments and strings."
  :lighter ""
  :keymap hl-todo-mode-map
  :group 'hl-todo
  (if hl-todo-mode
      (hl-todo--setup)
    (font-lock-remove-keywords nil hl-todo--keywords))
  (when font-lock-mode
    (if (and (fboundp 'font-lock-flush)
             (fboundp 'font-lock-ensure))
        (save-restriction
          (widen)
          (font-lock-flush)
          (font-lock-ensure))
      (with-no-warnings
        (font-lock-fontify-buffer)))))

But those are the functions added to do this sort of thing and I don't know what else I could do to make the new keywords take effect without implementing it myself. Even if these functions reliably did what I think they were designed to do, they would still be problematic because they refontify the whole buffer (when they actually do, which, as we have learned, doesn't appear to be always the case), then that would be wasteful (leading to issues such as #22).

What I would really like to be able to use is (font-lock-flush-and-ensure-additional-keywords-on-areas-which-have-already-been-fontified NEWLY-ADDED-KEYWORDS).

@tarsius
Copy link
Owner

tarsius commented Oct 3, 2018

Please try out #29.

@blaenk
Copy link
Author

blaenk commented Oct 7, 2018

Didn't seem to fix the issue. Tested with sql-mode.

Care to try it @dschrempf?

@tarsius
Copy link
Owner

tarsius commented Oct 8, 2018

I am unable to locate an sql-mode.el in Emacs or the Emacsmirror. Did you mean sql.el, which defines a mode sql-mode? Else please tell me where I can obtain sql-mode.el.

The version of sql-mode.el that comes with Emacs 26.1 contains four instances of XXX (and no other keywords), all of which are highlighted for me after hl-todo-mode is turned on using prog-mode (indirectly, using global-hl-todo-mode, without customizing hl-todo-activate-in-modes. Do you use another hook to activate hl-todo-mode? If so, please show me your complete setup for this feature.

@tarsius
Copy link
Owner

tarsius commented Oct 8, 2018

Oh! If you say "tested with sql-mode" you probably mean a buffer whose major-mode is sql-mode, right? If so please post such a file that I could use for testing.

@dschrempf I assume you were talking about latex-mode buffers, not a buffer visiting tex-mode.el. Did you customize hl-todo-activate-in-modes? latex-mode derives from tex-mode, which derives from text-mode, which is not a member of hl-todo-activate-in-modes by default.

@dschrempf
Copy link

Yes, I was talking about buffers using latex-mode. The help about the mode actually says: LaTeX/MPS mode defined in ‘tex-mode.el’.

As far as I can see the problem has not been fixed. I am using hl-todo-20181003.1521 from MELPA, I hope this is the correct version. Let me know, if I should specificly test it with this repository.

The value of hl-todo-avtivate-in-modes is (latex-mode prog-mode). I tried to change it to (TeX-latex-mode prog-mode), but that did not help either (also text-mode doesn't help).

Also hl-mode is reporting to be activated (but the keywords are only highlighted after deactivating it with subsequence activation):

hl-todo-mode is a variable defined in ‘hl-todo.el’.
Its value is t
Local in buffer MBE-PoMo.tex; global value is nil

  Automatically becomes buffer-local when set.

Documentation:
Non-nil if Hl-Todo mode is enabled.
Use the command ‘hl-todo-mode’ to change this variable.

[back]

I don't know why this is not working, it is working fine with emacs-lisp-mode and also other modes if I remember correctly. Maybe LaTeX mode is doing some highlighting itself? Thanks a lot for trying to fix this issue!

@tarsius tarsius closed this as completed in cdc2266 Oct 8, 2018
@tarsius
Copy link
Owner

tarsius commented Oct 8, 2018

I found the issue and fixed it in cdc2266.

@blaenk
Copy link
Author

blaenk commented Oct 8, 2018

Confirmed. Great work as usual! 👍

@dschrempf
Copy link

I am a little bit at a loss, because this still affects me in latex-mode. hl-todo-mode works in all other buffers as far as I can see. I latex-mode, a TODO keyword in a comment line has the following text properties

There are text properties here:
  face                 font-lock-comment-face
  font-lock-multiline  t
  fontified            t

The text properties of the same TODO keyword in a CPP buffer, where the highlighting works perfectly:

There are text properties here:
  c-in-sws             t
  face                 (:inherit hl-todo :foreground "#dc752f")
  fontified            t

If I disable and enable hl-todo-mode in the LaTeX buffer, the hihglighting works. However, when I save the file, the highlights vanish again!

@sachdevaprash
Copy link

I observe the same issue in latex-mode. After saving, I need to disable and re-enable hl-todo-mode.

@tarsius tarsius reopened this Sep 10, 2019
@tarsius
Copy link
Owner

tarsius commented Sep 19, 2019

this still affects me in
latex-mode. hl-todo-mode works in all other buffers as far as I
can see. I latex-mode, a TODO keyword in a comment line has the
following text properties

There are text properties here:
  face                 font-lock-comment-face
  font-lock-multiline  t
  fontified            t

I cannot reproduce that. For me the keywords are highlighted inside and outside comments. Are they highlighted outside of comments for you too?

In my case there is no mention of font-lock-multiline. I am guessing that the issue has something to do with that. I don't know why that is set for you but not for me. I only tested with Emacs 27.0.50 so far.

@dschrempf
Copy link

dschrempf commented Sep 20, 2019 via email

@tarsius
Copy link
Owner

tarsius commented Sep 24, 2019

You might want to try to reproduce the issue starting from emacs -Q.

@dschrempf
Copy link

After around two hours of debugging I could nail down the problem to loading the style file expl3.el, which is part of the expl3 package adding experimental LaTeX3 features. So, if I add

\usepackage{expl3}

AucTeX loads the style file of expl3.el, and hl-todo does not work. If I remove this line, everything is fine. Actually, I didn't load this package directly, but it was loaded as a dependency from other packages (such as the siunits package). I think that the reason is that the style file of expl3 makes : a symbol char, but this is a rough guess only, you may be much more proficient in this. I attach the expl3 style file, you may be more prorificient in understanding the contents (I had to rename it so that GitHub allows the upload).

expl3-style-file.txt

@tarsius
Copy link
Owner

tarsius commented Oct 2, 2019

I think that the reason is that the style file of expl3 makes : a symbol char, but this is a rough guess only, you may be much more proficient in this.

I would have to experiment too. Please post a minimal tex file that I can use for that once I get around to it. Meanwhile you can experiment too. Be commenting out parts of expl3.el (and then restarting Emacs to be on the save side) you can figure out if your guess was right. Figuring out whether it has to do with the syntax or the keyword part would be useful too.

@tarsius tarsius added the bug label Oct 2, 2019
@tarsius tarsius added wontfix and removed bug labels Oct 23, 2019
@tarsius
Copy link
Owner

tarsius commented Oct 23, 2019

On my system there is no expl3.el on load-path, I don't know how to install expl3 and why after adding \usepackage{expl3} to the file that causes AucTeX to load expl3.el. I am also getting confused about whether auctex is the same as tex-mode. Simply eval-buffer the expl3.el you provided does not work because LaTeX-mode-syntax-table is undefined for me. ... and so on ...

Too much about this is unknown to me and I do not want to learn the things I would have to learn to be able to eventually probably figure out that expl3.el is to blame because it does something that it shouldn't be doing.

I recommend that you comment out parts of expl3.el and then others until you know which part is to blame. Then learn about that part and figure out if it could instead be done in a way that does not interfere with hl-todo.

@tarsius tarsius closed this as completed Oct 23, 2019
@dschrempf
Copy link

dschrempf commented Oct 24, 2019 via email

@sachdevaprash
Copy link

I did some digging into this as well. I believe I may have found a lead. The lines that seem to be causing this in auctex are in the filefont-latex.el, in the function font-latex-update-font-lock. In auctex-12.1.2, that is at line 1287-88:

(setq font-lock-set-defaults nil)
(font-lock-set-defaults)

As I understand it, this clears out the font lock settings by hl-todo mode. Removing these lines seems to keep the font-locking done by hl-todo mode. Hopefully someone can find a reasonable fix based on this info. :) One way of fixing this might be to add an advice to font-lock-set-defaults. I don't use latex regularly anymore, but if there is some tests to quickly run, I am happy to help.

@sachdevaprash
Copy link

sachdevaprash commented Oct 26, 2019

Note that I had the same issue with outshine mode, and that too gets fixed by commenting out these two lines. I might submit this as a bug to auctex, where this should imo be fixed.

@sachdevaprash
Copy link

Adding to this - This should be a problem with the following packages probably: pythontex, expl3, fancyvrb, listings, comment, alltt, beamer, url, fancyhdr, minted.

@sachdevaprash
Copy link

Posted in AucTeX here: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=37945

@tarsius
Copy link
Owner

tarsius commented Oct 27, 2019

Thanks!

@dschrempf
Copy link

I just sent an inquiry on the status of this bug report since there seems to be no progress on the AucTeX side.

@tsdh
Copy link

tsdh commented Jun 10, 2020

Hi all,

I'm a co-maintainer of GNU AUCTeX and just looked at https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37945. Sorry for the inconvenience but that's indeed a quite hard problem. LaTeX is a beast where any adding or removing of a \usepackage{foo} might change fontification rules. That's why our foo-style might make a "full reset" if it changes the fontification rules.

I'll look into it, though...

@tsdh
Copy link

tsdh commented Jun 10, 2020

Just to let you all know, I'm working on fixing that issue in GNU AUCTeX.

@tsdh
Copy link

tsdh commented Jun 10, 2020

@dschrempf If you can, please try out the obsolete-font-latex-update-font-lock branch of AUCTeX and report back if that solves the issue for you (and hopefully doesn't break anything else).

http://git.savannah.gnu.org/cgit/auctex.git/log/?h=obsolete-font-latex-update-font-lock

@tsdh
Copy link

tsdh commented Jun 18, 2020

Just to report back, the issue is solved in GNU AUCTeX and will make it into the next ELPA release.

@tarsius
Copy link
Owner

tarsius commented Jun 18, 2020

Thanks!

@sachdevaprash
Copy link

sachdevaprash commented Jun 18, 2020

Amazing work @tsdh . Thank you!

@tsdh
Copy link

tsdh commented Jun 30, 2020

GNU AUCTeX 12.2.4 is released on ELPA and contains the fix for this issue.
https://elpa.gnu.org/packages/auctex.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants