-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add content_for
tag parser support + ValidContentForArguments
check
#568
Conversation
Tagging @albchu and @miazbikowski for context sharing. |
case NodeTypes.ContentForMarkup: { | ||
const contentForType = path.call((p: any) => print(p), 'contentForType'); | ||
const doc: Doc = [contentForType]; | ||
if (node.args.length > 0) { | ||
doc.push( | ||
',', | ||
line, | ||
join( | ||
[',', line], | ||
path.map((p) => print(p), 'args'), | ||
), | ||
); | ||
} | ||
|
||
return doc; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prettier logic is dense.
The gist is that line
is a space
when the doc doesn't break on a new line, and a new line + indentation
adjustment when the doc breaks.
What I'm writing here is that ContentForMarkup
nodes should be printed as follows
// no arguments?
{% content_for 'contentForType' %}
// arguments & no break ?
{% content_for 'contentForType', arg1: value1, arg2: value2 %}
// arguments + break ?
{% content_for 'contentForType',
arg1: value1,
arg2: value2
%}
const markup = node.markup; | ||
const trailingWhitespace = markup.args.length > 0 ? line : ' '; | ||
return tag(trailingWhitespace); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tag(trailingWhitespace)
helper is here to handle the logic about what "whitespace-logic-char" we want to use at the end of the tag.
// when it doesn't break
tag('') => {% content_for "blocks"%}
tag(' ') => {% content_for "blocks" %}
tag(line) => {% content_for "blocks" %}
// when it breaks
tag('') => {% content_for "blocks"%}
tag(' ') => {% content_for "blocks" %}
tag(line) => {% content_for "blocks"
%}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small stuff
...es/theme-language-server-common/src/completions/providers/RenderSnippetCompletionProvider.ts
Show resolved
Hide resolved
packages/theme-check-common/src/checks/valid-content-for-arguments/index.spec.ts
Show resolved
Hide resolved
packages/theme-check-common/src/checks/valid-content-for-arguments/index.ts
Show resolved
Hide resolved
packages/theme-check-common/src/checks/valid-content-for-arguments/index.ts
Show resolved
Hide resolved
3030bdc
to
f80a511
Compare
Fixes #466
What are you adding in this PR?
content_for
tagcontent_for
checks to use the AST rather than regexes to perform their workcontent_for
tagValidContentForArguments
check{% content_for "blocks" %}
context.*
arguments are accepted{% content_for "block" %}
type
andid
are requiredtype
andid
must be stringscontext.*
arguments are optionalWhat's next? Any followup issues?
ValidContentForArguments
on shopify.devcontent_for ""
first argument completion #569content_for "block"
named argument completion #570content_for "block"
type completion #571What did you learn?
There were more things I needed to change than I remembered. TypeScript helps a ton here because it complains any time you add a new node type.
Before you deploy
changeset
allChecks
array insrc/checks/index.ts
yarn build
and committed the updated configuration fileschangeset