Added doc tag support logic inside snippet tag #1067
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
shoplando
force-pushed
the
introduce-doc-tag-inside-snippet-tag
branch
2 times, most recently
from
4136c01 to
209fe79
Compare
|
|
||
| context.report({ | ||
| message: `The \`${docTagName}\` tag can only be used within a snippet or block.`, | ||
| message: `The \`${docTagName}\` tag can only be used at the top level of a snippet or block file, or as a direct child of an inline snippet tag.`, |
There was a problem hiding this comment.
(nit) Wondering if this could be clearer but nothing better is springing to mind. Might be worth workshopping a little bit to get "at the top level of" clearer.
There was a problem hiding this comment.
I totally agree, I'll try to come up with something clearer
There was a problem hiding this comment.
How does "The `${docTagName}` tag cannot be nested inside Liquid tags except for snippet tags." sound?
There was a problem hiding this comment.
Throwing in my two cents. Would it make sense to offer two different messages?
We keep the message for doc tags in snippets and blocks as The \${docTagName}\ tag can only be used at the top level of a snippet or block file
and then have a separate one triggered for inline snippets like `The ${docTagName} tag can only be used inside an inline snippet at the top level. (I don't have a good message for this)
My only reason for this is that the doc tags are now serving a purpose in different ways that would be confusing in one message.
We're top level only within blocks and snippet files but if we have an inline snippet that can be placed anywhere, it actually goes top level within the snippet itself.
There was a problem hiding this comment.
Hmm, yeah that's actually a great idea, I'll make two separate messages for the two cases
shoplando
force-pushed
the
introduce-doc-tag-inside-snippet-tag
branch
3 times, most recently
from
1c33a1e to
a47fc50
Compare
shoplando
force-pushed
the
introduce-doc-tag-inside-snippet-tag
branch
from
a47fc50 to
391578c
Compare
shoplando
changed the base branch from
include-snippet-in-ohm
to
graphite-base/1067
shoplando
changed the base branch from
graphite-base/1067
to
make-undefined-object-theme-check-accept-inline-snippets
shoplando
deleted the branch
make-undefined-object-theme-check-accept-inline-snippets
4 participants
What are you adding in this PR?
Solves #826
Updated theme check to support
doctag inside ofsnippettag. This change allows thedoctag to be used within inline snippets, in addition to the previously supported snippet and block contexts.This PR also makes the theme check more rigorous by ensuring the doc tag is either:
The error message has been updated to reflect this new support, and tests have been added to verify that
doctags inside inline snippets don't trigger errors.What's next? Any followup issues?
Update the theme check docs
Before you deploy
changeset