Add ability to disable theme check for next line #796
Conversation
58e6628 to
fe96774
Compare
| disabledRanges.set(check, []); | ||
| } | ||
|
|
||
| const nextNewLineIndex = source.slice(position.end).indexOf('\n', 1); |
There was a problem hiding this comment.
This is the most fragile piece of this approach. When we're in this part of the code the source is just a string. What I'm doing here is getting the index of the second new line by passing 1 as the second argument to indexOf. The very first character after the slice should be a new line which should return an index of 0 so we add 1 to that number to change the starting point of the search.
slice start slice end
v v
{% # theme-check-disable-next-line UnknownFilter %}(\n {{ product | broken }}\n{{ variable }})
^^ ^^
first new line next new line (end)
fe96774 to
2fe1178
Compare
|
|
||
| const offenses = await check({ 'code.liquid': file }, checks); | ||
|
|
||
| expect(offenses).toHaveLength(3); |
There was a problem hiding this comment.
This is kind of self documenting, but I had to go up to verify that the expected # of offenses is 3, and that the file value was similar to that of the previous tests. Maybe we could add a comment for that?
2fe1178 to
9ceddef
Compare
There was a problem hiding this comment.
This feels OK but can we add a test for errors that might squiggly on multiple lines?
If the disable-next-line is not entirely covering the error, is it omitted?
{% # theme-check-disable-next-line %}
{% render "snippet", argUnknown:
valueUndef
%}What if the squiggly is in a "pretty-formatted" thing but further down?
{% # theme-check-disable-next-line %}
{{ 'arg'
| unknown-filter
}}Shouldn't we try to target the next node in the tree instead? Or is this code (that I probably wrote) really not working like that?
There was a problem hiding this comment.
Shouldn't we try to target the next node in the tree instead? Or is this code (that I probably wrote) really not working like that?
@charlespwd I wish. I tired tweaking stuff a bit but by the time we get to the check we don't have that information. I'm going to have one more look at this though and make sure it wouldn't be too hard to also pass "next sibling node".
9ceddef to
9a44917
Compare
Force Push Patch Notes
|
What are you adding in this PR?
next-line#788Adds the ability to disable a theme check rule only on the next line with
theme-check-disable-next-line:{% # theme-check-disable-next-line UnknownFilter %} {{ product | foo }}Just like
theme-check-disableyou can pass no specific rule to disable all rules:{% # theme-check-disable-next-line %} {% assign x = product | foo %}Before you deploy
changeset