Multiple identical misspellings in same line have incorrect range |
||
Issue descriptionWhen a line contains the same misspelling two or more times, the spellchecker analyzer generates the appropriate comments, but they all highlight the first occurrence of the misspelling (see line 5 in misspellings.c here: https://chromium-review.googlesource.com/c/infra/infra/+/1166151). This is because of the usage of strings.Index to find the misspelled word in the line (which always returns the first occurrence of the word). To fix, we need to keep track of the index of the words being checked in the line and specify that as the startChar. Also, the endChar passed into buildMisspellingComment isn't needed, since it's just startChar + len(misspelling).
,
Aug 17
Hi, Can I take this issue? I am new here, this seems easy to begin with. :)
,
Aug 17
Sure! - This is regarding the spellchecker that produces comments posted to code review; the code is at https://cs.chromium.org/chromium/infra/go/src/infra/tricium/functions/spellchecker/spellchecker.go. To get set up to make changes in the infra repo, see https://chromium.googlesource.com/infra/infra/+/master/go/#get-the-code.
,
Sep 4
The following revision refers to this bug: https://chromium.googlesource.com/infra/infra/+/9f31acd685a03a1e2360799a313eee225ad7e813 commit 9f31acd685a03a1e2360799a313eee225ad7e813 Author: Parminder Singh <parmsingh129@gmail.com> Date: Tue Sep 04 13:41:06 2018 Handling same spelling mistakes in same line in tricium Instead of just getting fields, the index is retrieved. This is used for calculating startChar step by step. This allows handling the position of misspellings in a more reliable way. Did some other clean code tweaks too: Comments object made common between processComment and analyse comments and passed as reference for cleaner interface Removed endChar's need in buildMisspellingComment Removed line variable dependency in analyseWords and processCommandWord Bug: 873272 Change-Id: I16802e42180e2f24bdfa5806e4325ffb8b748979 Reviewed-on: https://chromium-review.googlesource.com/1180626 Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org> Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org> Cr-Commit-Position: refs/heads/master@{#17376} [modify] https://crrev.com/9f31acd685a03a1e2360799a313eee225ad7e813/go/src/infra/tricium/functions/spellchecker/spellchecker_test.go [modify] https://crrev.com/9f31acd685a03a1e2360799a313eee225ad7e813/go/src/infra/tricium/functions/spellchecker/test/example.c [modify] https://crrev.com/9f31acd685a03a1e2360799a313eee225ad7e813/go/src/infra/tricium/functions/spellchecker/spellchecker.go
,
Sep 4
The following revision refers to this bug: https://chromium.googlesource.com/infra/infra/+/9f31acd685a03a1e2360799a313eee225ad7e813 commit 9f31acd685a03a1e2360799a313eee225ad7e813 Author: Parminder Singh <parmsingh129@gmail.com> Date: Tue Sep 04 13:41:06 2018 Handling same spelling mistakes in same line in tricium Instead of just getting fields, the index is retrieved. This is used for calculating startChar step by step. This allows handling the position of misspellings in a more reliable way. Did some other clean code tweaks too: Comments object made common between processComment and analyse comments and passed as reference for cleaner interface Removed endChar's need in buildMisspellingComment Removed line variable dependency in analyseWords and processCommandWord Bug: 873272 Change-Id: I16802e42180e2f24bdfa5806e4325ffb8b748979 Reviewed-on: https://chromium-review.googlesource.com/1180626 Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org> Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org> Cr-Commit-Position: refs/heads/master@{#17376} [modify] https://crrev.com/9f31acd685a03a1e2360799a313eee225ad7e813/go/src/infra/tricium/functions/spellchecker/spellchecker_test.go [modify] https://crrev.com/9f31acd685a03a1e2360799a313eee225ad7e813/go/src/infra/tricium/functions/spellchecker/test/example.c [modify] https://crrev.com/9f31acd685a03a1e2360799a313eee225ad7e813/go/src/infra/tricium/functions/spellchecker/spellchecker.go
,
Sep 4
The following revision refers to this bug: https://chromium.googlesource.com/infra/infra/+/9f31acd685a03a1e2360799a313eee225ad7e813 commit 9f31acd685a03a1e2360799a313eee225ad7e813 Author: Parminder Singh <parmsingh129@gmail.com> Date: Tue Sep 04 13:41:06 2018 Handling same spelling mistakes in same line in tricium Instead of just getting fields, the index is retrieved. This is used for calculating startChar step by step. This allows handling the position of misspellings in a more reliable way. Did some other clean code tweaks too: Comments object made common between processComment and analyse comments and passed as reference for cleaner interface Removed endChar's need in buildMisspellingComment Removed line variable dependency in analyseWords and processCommandWord Bug: 873272 Change-Id: I16802e42180e2f24bdfa5806e4325ffb8b748979 Reviewed-on: https://chromium-review.googlesource.com/1180626 Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org> Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org> Cr-Commit-Position: refs/heads/master@{#17376} [modify] https://crrev.com/9f31acd685a03a1e2360799a313eee225ad7e813/go/src/infra/tricium/functions/spellchecker/spellchecker_test.go [modify] https://crrev.com/9f31acd685a03a1e2360799a313eee225ad7e813/go/src/infra/tricium/functions/spellchecker/test/example.c [modify] https://crrev.com/9f31acd685a03a1e2360799a313eee225ad7e813/go/src/infra/tricium/functions/spellchecker/spellchecker.go
,
Oct 4
This has been fixed by Parminder in the above patch (verified in https://chromium-review.googlesource.com/c/playground/gerrit-tricium/+/1246468/4) |
||
►
Sign in to add a comment |
||
Comment 1 by qyears...@chromium.org
, Aug 10Status: Available (was: Untriaged)