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

Wrong font lock used when highlighting an element containing a string with punctuation inside #82

Closed
codrut-fc opened this issue Sep 1, 2020 · 27 comments

Comments

@codrut-fc
Copy link

codrut-fc commented Sep 1, 2020

Screen Shot 2020-09-01 at 14 32 03

This bug:

  • happens after line 46 (for some reason).
  • triggers when dealing with an array, having a quoted string that contains numbers and a colon punctuation.
  • "fixes" itself temporarily by deleting the last quote, saving, undoing, then saving again; reappears when reloading the file.
  • the highlighting of the rest of the file underneath is messed up by it.

This is especially troublesome with docker-compose.yml file where you define port mappings like:

services:
  grafana:
    image: grafana/grafana
    ports:
      - '3000:3000'

I have no idea how to investigate/fix this myself, so I'm asking for help (please) 🙏 !

EDIT: see #82 (comment) below for an investigation done by @sulami.

@eeshugerman
Copy link

eeshugerman commented Sep 13, 2020

Glad it's not just me, I run into this a lot. I don't think line 46 is special in general, just in your example. Here's the simplest case I've managed to find:

a: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
a: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
a: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
a: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
a: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
a: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
a: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
a: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
a: 'a.a'
a: 'a'

image

(issue is present on the last line)

EDIT: just realized there needn't be quotes on the last line to observe the issue

@codrut-fc codrut-fc changed the title Wrong font lock used after line 46 when highlighting an array element containing a string with numbers and a colon inside Wrong font lock used when highlighting an array element containing a string with punctuation inside Sep 14, 2020
@codrut-fc codrut-fc changed the title Wrong font lock used when highlighting an array element containing a string with punctuation inside Wrong font lock used when highlighting an element containing a string with punctuation inside Sep 14, 2020
@codrut-fc
Copy link
Author

Thanks for providing another test case @eeshugerman, I've updated the title and description to better fit both scenarios.

@eeshugerman
Copy link

It seems this bit is to blame -- if I remove it, this issue is resolved.

Of course, removing it re-introduces the issue that it's there to solve (see the comment).

Unless someone can get to the bottom of this one, I'd argue it makes sense to remove it. IMO the case that it improves (quotes in unquoted strings) is more of an edge-case than the case that it breaks (punctuation in quoted strings), and (all else equal) less code is better than more code.

@wasamasa, thoughts?

@wasamasa
Copy link
Collaborator

My thought on this is that I'm not going to remove code introduced by @dgutov (it's easily the most sophisticated contribution so far and he's a core Emacs developer), so battle it out with him.

@dgutov
Copy link
Contributor

dgutov commented Nov 20, 2020

Two things:

So... I would suggest two avenues for research: a) try checking out that older version and see whether the problems is still there, b) some print-debugging, I guess?

Also try some new version of Emacs (which ones are you seeing the problem with?) and running it with -Q first, to establish the full reproduction scenario.

@eeshugerman
Copy link

Thanks you two for chiming in! Sure enough, I do not observe the issue with -Q. (-Q is new to me btw; I'll be sure to try it next time I run into a bug.) Also I should mention that even without -Q, #82 (comment) isn't a surefire way to reproduce the issue -- I have to mess with it1 to trigger it. But no matter what I do with -Q it doesn't happen.

I'm using Spacemacs, so I suppose there are a lot of places this could be going awry 😕. @codrut-fc are you using Spacemacs perchance? And/or Evil?

I'm using v27.1. IIRC I saw the issue on v26.3 as well. I'll look into installing v28/master.


  1. M-x yaml-mode after pasting (yanking) the example into the buffer, even if the buffer already had yaml-mode enabled, seems to reproduce the issue consistently. Ditto for pasting from evil-emacs-state (vs evil-normal-state, as I would normally). And ditto for pasting the example with one or more empty lines before it. Etcetera...

@codrut-fc
Copy link
Author

codrut-fc commented Nov 20, 2020

@eeshugerman yes, using Spacemacs:

#### System Info
- OS: darwin
- Emacs: 28.0.50
- Spacemacs: 0.300.0
- Spacemacs branch: develop (rev. 812877409)
- Graphic display: nil
- Distribution: spacemacs
- Editing style: vim
- Completion: helm

Config file available here: https://gitlab.com/sdwolfz/dotfiles/-/blob/master/.spacemacs

@dgutov
Copy link
Contributor

dgutov commented Nov 20, 2020

M-x yaml-mode after pasting (yanking) the example into the buffer

That was one of the first things I tried. ;-(

If Spacemacs is what triggers it, someone should bisect its configuration. Either of you can do it, or you can file an issue at the Spacemacs issue tracker, I guess...

@codrut-fc
Copy link
Author

I've actually looked through the Spacemacs source and I could not find anything that touches yaml-mode internals. Will try again, maybe it's not Spacemacs itself but another package installed somewhere (ansible comes to mind as it relies on yaml-mode)

@dgutov
Copy link
Contributor

dgutov commented Nov 20, 2020

Could be something else that's generally font-lock related, for example.

If you can comment out whole parts of the Spacemacs configuration at a time, that should make for a faster search.

@codrut-fc
Copy link
Author

@dgutov do you have any tips on "live debugging" this as the file loads? I can't seem to figure out a way to visualise and understand what's happening.

@dgutov
Copy link
Contributor

dgutov commented Nov 20, 2020

You could try https://github.com/Lindydancer/font-lock-studio, I suppose. Or print-debugging (meaning adding some message calls around the yaml-mode's font-lock code). But both require a certain level of understanding of font-lock.

I do believe you would be better served by bisecting the config, to find the smallest possible configuration where the bug can be reproduced first, either way.

@codrut-fc
Copy link
Author

I'll close this issue for now, hopefully I'll have time to investigate more into this soon...

@sulami
Copy link

sulami commented Jan 12, 2021

Picking this up because it has been annoying me for a while.

When using the example from above, I find it interesting that any editing of the buffer fixes the fontification, as does font-lock-debug-fontify. I can only reproduce this by pasting in the entire sample as-is, not by typing it out.

I haven't looked very deep into this just now, but I'm wondering if this could be a situation where several properties can match and the order they're applied in matters. This would mean that any incremental edit would trigger a different order of application than a loading from scratch, or something along those lines.

@dgutov
Copy link
Contributor

dgutov commented Jan 12, 2021

Properties? Syntax-propertize rules?

I can understand that it's annoying, but we're unlikely to make much progress without having a simple repro scenario starting with a base Emacs session.

@eeshugerman
Copy link

@sulami, what's your config like? are you using spacemacs?

@sulami
Copy link

sulami commented Jan 12, 2021 via email

@codrut-fc
Copy link
Author

OK, I was under the impression that this was a Spacemacs specific bug, but since @sulami confimed it happens outside of it then do you want me to repoen the issue?

@sulami
Copy link

sulami commented Jan 12, 2021 via email

@codrut-fc codrut-fc reopened this Jan 12, 2021
@sdwolfz
Copy link

sdwolfz commented Jan 12, 2021

Pinging @tarsius, maybe he has some quick insight to share (I personally can't figure it out).

@tarsius
Copy link
Contributor

tarsius commented Jan 13, 2021

I don't immediately see what might be causing this.

@codrut-fc
Copy link
Author

Note: I can also confirm that excluding the hl-todo package in Spacemacs makes the issue go away. Now the hunt begins!

@runejuhl
Copy link

I'm not using Spacemacs, but have been bugged by this same issue. Disabling hl-todo indeed does make the issue go away. 🤞 that you figure it out :)

@dgutov
Copy link
Contributor

dgutov commented Jan 13, 2021

Thanks for finding the conflict, I'll try to look into it this weekend.

@dgutov
Copy link
Contributor

dgutov commented Jan 17, 2021

@tarsius

If I remove hl-todo--syntax-table and the associated with-syntax-table call, the problem doesn't occur. It seems regexp search (or most movements of point, really) can trigger syntax-propertize-function invocation, and when that happens while an unexpected syntax table is in effect, weird things happen.

Alternatively, adding this extra item to the let-binding above it seems to also do the trick:

           (let ((case-fold-search nil)
                 (syntax-ppss-table (syntax-table)))
             (with-syntax-table hl-todo--syntax-table
               (funcall (if backward #'re-search-backward #'re-search-forward)
                        regexp bound t)))

syntax-ppss-table was only added in Emacs 26, though.

tarsius added a commit to tarsius/hl-todo that referenced this issue Jan 17, 2021
It seems regexp search (or most movements of point, really) can
trigger syntax-propertize-function invocation, and when that happens
while an unexpected syntax table is in effect, weird things happen.

Fixes yoshiki/yaml-mode#82.

Suggested-by: Dmitry Gutov <dgutov@yandex.ru>
@tarsius
Copy link
Contributor

tarsius commented Jan 17, 2021

I have applied that work around. Thanks @dgutov!

@sdwolfz
Copy link

sdwolfz commented Jan 17, 2021

I can confirm this is fixed now, Thank you very much for looking into this!

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

No branches or pull requests

8 participants