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

js2-mode: Comments are colorized #40

Closed
FrancisMurillo opened this issue Jun 9, 2016 · 8 comments · Fixed by #93
Closed

js2-mode: Comments are colorized #40

FrancisMurillo opened this issue Jun 9, 2016 · 8 comments · Fixed by #93

Comments

@FrancisMurillo
Copy link

Is it correct that comments are also colorized?

Great package by the way, helps me see the variables better.

@11111000000
Copy link

it's colorized after color-identifiers:refresh , but not immediately

@ankurdave
Copy link
Owner

I think js2-mode sometimes removes the face from comments briefly, causing color-identifiers:scan-identifiers to think the words are valid candidate identifiers.

The right fix is to have two modes for scanning identifiers, one for identifier discovery and another for actually coloring. In the discovery mode, only candidates already fontified with with font-lock-variable-name-face or js2-function-param should be considered. In the coloring mode, unfontified text should also be considered.

@bennofs
Copy link

bennofs commented May 7, 2019

This is caused by mooz/js2-mode#450

Hi-Angel added a commit to Hi-Angel/color-identifiers-mode that referenced this issue Jan 19, 2023
Hi-Angel added a commit to Hi-Angel/color-identifiers-mode that referenced this issue Jan 19, 2023
@Hi-Angel
Copy link
Collaborator

@bennofs please give a test to #93. That fixes the problem for me, however as I don't code in JS myself, I can't see offhand if that doesn't cause unwanted side-effects. It should not though, and in my testing everything seem to look okay.

@Hi-Angel
Copy link
Collaborator

Oh, sorry, the report was opened by @FrancisMurillo. So, anyway, please see if the PR fixes the problem, it's really just a two line change which can even be stored in .emacs file.

Hi-Angel added a commit to Hi-Angel/color-identifiers-mode that referenced this issue Jan 19, 2023
I tried to dig why the difference compared to the default
`color-identifiers:scan-identifiers`, but it already took a lot of time
due to having to figure out the algorithms and cases where one or the
other is called *(so e.g. `color-identifiers:scan-identifiers` is called
always even if the cc-mode is set, just from other places)*. I have a
guess about roots of the problem, but fixing that would require more
time, so let's have this easy fix for now instead.

Fixes: ankurdave#40
@Hi-Angel
Copy link
Collaborator

So, yeah, just to be clear: to test it you can simply paste the two added lines into your .emacs file, should work just fine.

@Hi-Angel
Copy link
Collaborator

Hi-Angel commented Jan 19, 2023

Huh, actually, the commit Don't go to (point-min) just to immediately relocate to property-change from this PR fixes that for me as well o.O Who would have thought…

Now that I think of that, the reason is probably that when buffer has no font-locks, the (next-property-change (point)) will return nil, so the complete function is basically skipped.

@Hi-Angel
Copy link
Collaborator

Huh, actually, the commit Don't go to (point-min) just to immediately relocate to property-change from this PR fixes that for me as well o.O Who would have thought…

Nvm, I dropped the change from that commit for now. I don't fully understand why (scan-identifiers) works the way it does, so am afraid it might break something. Not gonna touch it for now.

Hi-Angel added a commit to Hi-Angel/color-identifiers-mode that referenced this issue Jan 19, 2023
`scan-identifiers` isn't the best scan-fn (see also ankurdave#94) as it moves not
just through changed properties. Let's use the cc-mode one, which goes
exclusively through face changes while searching for declared
identifiers.

Fixes: ankurdave#40
Hi-Angel added a commit that referenced this issue Jan 19, 2023
`scan-identifiers` isn't the best scan-fn (see also #94) as it moves not
just through changed properties. Let's use the cc-mode one, which goes
exclusively through face changes while searching for declared
identifiers.

Fixes: #40
Hi-Angel added a commit to Hi-Angel/color-identifiers-mode that referenced this issue Jan 22, 2023
It isn't actually specific to cc-mode, and works in a way similar to the
previous default `scan-identifiers`, except that because it only goes
through places with properties set, it is:

1. more performant due to less motion
2. less likely to introduce wrong coloring as in ankurdave#40 or ankurdave#62

We also rename the function to remove infix `cc-mode` as it isn't (and
never really have been) specific to c-mode.

Fixes: ankurdave#94
Hi-Angel added a commit to Hi-Angel/color-identifiers-mode that referenced this issue Jan 22, 2023
It isn't actually specific to cc-mode, and works in a way similar to the
previous default `scan-identifiers`, except that because it only goes
through places with properties set, it is:

1. more performant due to less motion
2. less likely to introduce wrong coloring as in ankurdave#40 or ankurdave#62

We also rename the function to remove infix `cc-mode` as it isn't (and
never really have been) specific to c-mode.

Fixes: ankurdave#94
Hi-Angel added a commit to Hi-Angel/color-identifiers-mode that referenced this issue Jan 22, 2023
It isn't actually specific to cc-mode, and works in a way similar to the
previous default `scan-identifiers`, except that because it only goes
through places with properties set, it is:

1. more performant due to less motion
2. less likely to introduce wrong coloring as in ankurdave#40 or ankurdave#62

We also rename the function to remove infix `cc-mode` as it isn't (and
never really have been) specific to c-mode.

Fixes: ankurdave#94
Hi-Angel added a commit that referenced this issue Jan 23, 2023
It isn't actually specific to cc-mode, and works in a way similar to the
previous default `scan-identifiers`, except that because it only goes
through places with properties set, it is:

1. more performant due to less motion
2. less likely to introduce wrong coloring as in #40 or #62

We also rename the function to remove infix `cc-mode` as it isn't (and
never really have been) specific to c-mode.

Fixes: #94
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

Successfully merging a pull request may close this issue.

5 participants