Fix warnings and small mistakes in several lexers #1679
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Occurrences: * lib/rouge/lexers/clean.rb:82: warning: character class has duplicated range: /_*[a-z][\w_`]*/ * lib/rouge/lexers/clean.rb:90: warning: character class has duplicated range: /_*[A-Z][\w_`]*/ * lib/rouge/lexers/clean.rb:98: warning: character class has duplicated range: /[^\w_\s`]/ * lib/rouge/lexers/clean.rb:139: warning: character class has duplicated range: /[\w_]+/ Reason: * `\w` already includes `_` so specifying both is unnecessary
Occurrences:
* lib/rouge/lexers/ecl.rb:117: warning: character class has duplicated range: /[:=|>|<|<>|\/|\\|\+|-|=]/
* lib/rouge/lexers/ecl.rb:118: warning: character class has duplicated range: /[\[\]{}();,\&,\.,\%]/
Reason:
* `[]` used when not desired (also precedence order was incorrect so `<>` would never match)
* `,` used multiple times inside the same `[]`
Occurrences: * lib/rouge/lexers/gdscript.rb:31: warning: assigned but unused variable - builtins
Occurrences: * lib/rouge/lexers/ghc_cmm.rb:25: warning: character class has duplicated range: /(?!#[a-zA-Z])[\w#\$%_']+/ * lib/rouge/lexers/ghc_cmm.rb:30: warning: character class has duplicated range Reason: * `\w` already includes `_` so specifying both is unnecessary
Occurrences: * lib/rouge/lexers/sql.rb:118: warning: character class has duplicated range: /\w[\w\d]*/ Reason: * `\w` already includes `\d` so specifying both is unnecessary * `\w` initial token was defined incorrectly (should not start with a number)
Occurrences:
* lib/rouge/lexers/ada.rb:158: warning: character class has duplicated range: /\b(\p{Pc}|[[alpha]])\p{Word}*/
Reason:
* `[[alpha]]` was missing semicolons and therefore not being evaluated as a POSIX bracket expression
Occurrences:
* lib/rouge/lexers/kotlin.rb:27: warning: character class has duplicated range
* lib/rouge/lexers/kotlin.rb:29: warning: character class has duplicated range
* lib/rouge/lexers/kotlin.rb:30: warning: character class has duplicated range
* lib/rouge/lexers/kotlin.rb:56: warning: character class has duplicated range
* lib/rouge/lexers/kotlin.rb:87: warning: character class has duplicated range
* lib/rouge/lexers/kotlin.rb:88: warning: character class has duplicated range
* lib/rouge/lexers/kotlin.rb:93: warning: character class has duplicated range
* lib/rouge/lexers/kotlin.rb:94: warning: character class has duplicated range
* lib/rouge/lexers/kotlin.rb:109: warning: character class has duplicated range
Reason:
* `\p{Pc}` already include `_` so specifying both is unnecessary
Occurrences: * lib/rouge/lexers/objective_c/common.rb:7: warning: assigned but unused variable - id Reason: * content on line 7 was duplicated on line 57
Occurrences: * lib/rouge/lexers/solidity.rb:14: warning: character class has duplicated range: /[a-zA-Z$_][\w$_]*/ Reason: * `\w` already includes `_` so specifying both is unnecessary
Occurrences: * rouge/lib/rouge/lexers/yang.rb:13: warning: character class has duplicated range: /[\w_-]+(?=[^\w\-\:])\b/ Reason: * `\w` already includes `_` so specifying both is unnecessary
dblessing
reviewed
Feb 22, 2021
| @@ -115,7 +115,7 @@ def self.keywords_type | |||
| rule %r/"/, Name::Variable, :double_string | |||
| rule %r/`/, Name::Variable, :backtick | |||
|
|
|||
| rule %r/\w[\w\d]*/ do |m| | |||
| rule %r/[[:alpha:]][\w]*/ do |m| | |||
There was a problem hiding this comment.
We lose the digit piece here. Did you decide that's unnecessary since keyword_types don't include any digits? Similarly, :alpha: has slightly different meaning that \w - why the change? I actually wonder why we need both pieces anyway. Isn't %r/\w+/ equivalent? I'm slightly hesitant, though, as maybe there's something I'm not considering. Appreciate your insight.
tancnle
added
the
author-action
|
@brodock @dblessing Ping! Any chance to get this rebased and rolled out? There are so many warnings due to rouge now that it's basically down to either removing rouge (or tty-markdown pulling it in my case, ouch) or running tests without warnings which is not a good idea neither. Thanks a lot for your work on this!! |
|
@brodock @dblessing @svoop This has been resolved by #2030. Thanks a lot for your effort here @brodock 🙇🏼 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When running tests for a gem that depends on
rougeI've noticed that there are a bunch of relatively easy to fix warnings popping up during the test execution.I've split into separate commits each warning / fix per file required and added an explanation on each commit message of what was the problem.
When evaluating each regular expression issue, I've used Ruby 2.0.0 Regexp documentation as base.
To reproduce the warnings above, I've run specs with
-W2option enabled: