search

veripool / verilog-mode

Verilog-Mode for Emacs with Indentation, Hightlighting and AUTOs. Master repository for pushing to GNU, verilog.com and veripool.org.
http://veripool.org/verilog-mode
GNU General Public License v3.0
247 stars 90 forks source link

Incorrect highlighting of words after "module" or "interface" in a comment #1753

Closed fnJeff closed 1 year ago

fnJeff commented 2 years ago

Another highlighting issue I noticed. It appears that if the word "module" of "iterface" appears in a comment, the word after it is highlighted using a particular face. This seems to be the same face used after module/interface in non-comment context. There are probably other related keywords that have this same effect, but these are the two I found hiding my existing code so far.

image

wsnyder commented 2 years ago

verilog-font-lock-keywords-1 is what highlights this. Normally the syntax table indicates in a comment so overrides this, not sure why it isn't.

gmlarumbe commented 2 years ago

Hi,

This should be fixed in 883179f

According to https://github.com/Lindydancer/presentations/blob/master/font-lock-introduction.org:

Font-lock keywords Flags

A HIGHLIGHT has two optional flag fields:

(SUBEXP FACE OVERRIDE LAXMATCH) OVERRIDE

OVERRIDE controls what happens when there already was a face at the location. If not present, the match is not applied. In most cases this is the desired effect, as we don’t want to mess up comments or strings. Legal values are “t”, “keep”, “append”, and “prepend”.

Hint: Prefer “prepend” over “t”, as it behaves better when the new and existing face provides different attributes, for example when the existing face specifies a background color and the new doesn’t.

For some reason it was set to prepend and was overriding comments and strings.

Test snippet:

// Face: `font-lock-function-name-face'
// module foo
// primitive foo
// class foo
// program foo
// interface foo
// package foo
// task foo
//
// Face: `font-lock-constant-face'
// function real foo
// function integer foo
// function time foo

Before:

verilog_font_lock_comments

After:

verilog_font_lock_comments2

gmlarumbe commented 2 years ago

@fnJeff can you confirm it works for you as well?

gmlarumbe commented 2 years ago

@wsnyder , does it work for you?

I would probably make a PR but it contains very few changes and commit doesn't affect any test.

wsnyder commented 2 years ago

If you want me to merge it, I'll need a pull request, thanks.

gmlarumbe commented 2 years ago

I'm sorry, I got a bit confused wrt this issue.

I already pushed the changes into master in 883179f (3 weeks ago). I just wanted to make sure that it works fine also under XEmacs before closing the issue.

You can try with previous snippet:

// Face: `font-lock-function-name-face'
// module foo
// primitive foo
// class foo
// program foo
// interface foo
// package foo
// task foo
//
// Face: `font-lock-constant-face'
// function real foo
// function integer foo
// function time foo
fnJeff commented 1 year ago

Apologies, I was buried at work followed by vacation followed by reburying at work.

Hopefully you were able to verify this well enough without me.

Thank you for the fix! I look forward to it whenever I get the time to update to a newer version.

fnJeff

On Sun, Aug 7, 2022 at 5:46 PM Gonzalo Larumbe @.***> wrote:

Closed #1753 https://github.com/veripool/verilog-mode/issues/1753 as completed.

— Reply to this email directly, view it on GitHub https://github.com/veripool/verilog-mode/issues/1753#event-7142387691, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHOMRNILMBLQZ734G3AOEYDVX7K6RANCNFSM5H4U4RMQ . You are receiving this because you were mentioned.Message ID: @.***>

fnJeff commented 1 year ago

Finally had time to update to the latest version. Apologies for the ridiculous delay. Confirmed as fixed. Thank you for this fix!!!