New issue
Advanced search Search tips

Issue 877486 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 26
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Issue report for SpellChecker - DONT in .gn file noisy false positives

Project Member Reported by metzman@chromium.org, Aug 24

Issue description

https://tricium-prod.appspot.com/run/5062833058349056

Thank you for Tricium, it is an interesting project and I'm excited to see it mature.

It had some problems on this CL: https://chromium-review.googlesource.com/c/chromium/src/+/1187835

1. I don't think Tricium should check for punctuation marks within a word that has underscores. In this particular case Tricium is telling me my environment variables don't have an apostrophe in the word "DONT".

2. Reporting the same issue each time I upload is pretty annoying. I think highlighting spelling issues once per line is enough if that line is unchanged.

I also think some kind of limit on number of issues raised per round of review is a good idea while there are False Positives.

Thanks!
 
Cc: -diegomtzg@google.com mar...@chromium.org tandrii@chromium.org
Status: Available (was: Untriaged)
Summary: Issue report for SpellChecker - DONT in .gn file noisy false positives (was: Issue report for SpellChecker)
Thanks for the report, very useful feedback.

In this case, the easiest short-term fix would be to whitelist "dont" and "wont" and other such words in the spellchecker; limiting the number of reports for the same word sounds like a good idea.

Also, the spellchecker is only supposed to leave warnings for comments, so it seems to be misbehaving in this case (.gn is listed in https://cs.chromium.org/chromium/infra/go/src/infra/tricium/functions/spellchecker/comment_formats.json, so it should only check in comments starting with #).

This week I just started leave, so I can't work on this in the near future though, sorry :-/
I thought the repeated comments had been addressed by only reporting each misspelling at most once but it looks like it wasn't done (?)
Owner: qyears...@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 25

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/7de6b411b172d222579dddbed67188772e9661ab

commit 7de6b411b172d222579dddbed67188772e9661ab
Author: Quinten Yearsley <qyearsley@chromium.org>
Date: Tue Sep 25 19:57:43 2018

[tricium spellchecker] Fix handling of block comment state

The current behavior of spellchecker was that if there is
no block comment pattern in the comment formats file, then
everything is considered to be in a block comment (since
block comment start is empty string if not specified).

This fixes it so that if block comment start is empty,
we should never enter block comment state.

Bug:  877486 
Change-Id: Iebcd42d0231abe686d2580ad66e407b56c8a19ad
Reviewed-on: https://chromium-review.googlesource.com/1243681
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#17869}
[modify] https://crrev.com/7de6b411b172d222579dddbed67188772e9661ab/go/src/infra/tricium/functions/spellchecker/test/tricium/data/files.json
[modify] https://crrev.com/7de6b411b172d222579dddbed67188772e9661ab/go/src/infra/tricium/functions/spellchecker/spellchecker.go
[add] https://crrev.com/7de6b411b172d222579dddbed67188772e9661ab/go/src/infra/tricium/functions/spellchecker/test/BUILD.gn
[modify] https://crrev.com/7de6b411b172d222579dddbed67188772e9661ab/go/src/infra/tricium/functions/spellchecker/dictionary.txt

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 25

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/43a716d039f47b4213703029c2d160761fdd7f54

commit 43a716d039f47b4213703029c2d160761fdd7f54
Author: Quinten Yearsley <qyearsley@chromium.org>
Date: Tue Sep 25 20:32:32 2018

[tricium spellchecker] Don't check words in all-caps

Bug:  877486 
Change-Id: I7bad919db43e9703f7bd6016ea8a3364f56b92a5
Reviewed-on: https://chromium-review.googlesource.com/1244276
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#17871}
[modify] https://crrev.com/43a716d039f47b4213703029c2d160761fdd7f54/go/src/infra/tricium/functions/spellchecker/spellchecker_test.go
[modify] https://crrev.com/43a716d039f47b4213703029c2d160761fdd7f54/go/src/infra/tricium/functions/spellchecker/spellchecker.go

Status: Fixed (was: Assigned)
Now, words in all caps aren't checked, and words outside of comments in .gn files aren't checked; test CL https://chromium-review.googlesource.com/c/playground/gerrit-tricium/+/1246468/3.

Still to do: Not repeating the same comment multiple times; filed bug 889541.

Sign in to add a comment