[LiquidDoc] Parser support for optional parameters #733

jamesmengo · 2025-01-24T22:35:24Z

What are you adding in this PR?

Part of https://github.com/Shopify/develop-advanced-edits/issues/525

Adds LiquidDoc parsing support to allow users to define optional parameters.

Optional Parameters - @param [optional_param]

By default, parameters are now required unless they have [] around the name
Parameters with incomplete delimiters e.g. ([missingTail) will map to a TextNode

Adds parser support for [ and ] delimiters around the stringName, which indicate that a param is optional

Within the code, I'm storing this as 'required' rather than 'optional'. The default will be required: true

@param [paramName]

What's next? Any followup issues?

Before you deploy

I included a minor bump changeset
My feature is backward compatible

jamesmengo · 2025-01-24T22:35:43Z

[LiquidDoc] Hover: Wrap optional parameter names in [] #735
[LiquidDoc] Formatting support for optional @param delimiters [] #734
[LiquidDoc] Parser support for optional parameters #733 👈 (View in Graphite)
[LiquidDoc] Prevent rendering of hover tooltip for snippets with no renderable content #729
[LiquidDoc] Add snippet hover support inside of {% render %} tag #703 : 2 other dependent PRs (#719 , #726 )
Add parsing + prettier support for @param in {% doc %} tags #646 : 3 other dependent PRs (#652 , #665 , #691 )
Add basic {% doc %} tag parsing support #621 : 2 other dependent PRs (#636 , #638 )
main

This stack of pull requests is managed by Graphite. Learn more about stacking.

packages/liquid-html-parser/grammar/liquid-html.ohm

packages/liquid-html-parser/src/stage-1-cst.spec.ts

packages/liquid-html-parser/src/stage-1-cst.ts

packages/liquid-html-parser/grammar/liquid-html.ohm

packages/liquid-html-parser/src/stage-1-cst.spec.ts

packages/liquid-html-parser/src/stage-1-cst.ts

graygilmore

Did not get a 🎩 in yet but since this is just parser changes maybe the tests are all we need here!

.changeset/smart-onions-pump.md

packages/liquid-html-parser/grammar/liquid-html.ohm

jamesmengo · 2025-02-04T02:50:12Z

Closing in favour of #753

jamesmengo · 2025-02-04T20:41:27Z

packages/liquid-html-parser/src/stage-1-cst.ts

  paramDescription: ConcreteTextNode | null;
  paramType: ConcreteTextNode | null;
 }

+export interface ConcreteLiquidDocParamNameNode


It may look kind of weird to have required live in this node, however this is consistent with how the parsing logic works, and allows us to use the mappings in stage-1.

We do this because it aligns with the parsing grammar, and allows us to leverage the Mappings mechanism. We map this to a more sensible location in stage-2 here (node.paramName.required -> node.required)

---- - Added a parameter `required` to the LiquidDocParamNode - Parameters with `[]` around the name will return `required: true` - Parameters with incomplete delimiters `e.g. ([missingTail)` will map to a `TextNode`

aswamy

Looks great! Can't wait for prettier PR!

jamesmengo changed the title ~~Add optional param delimiters to ohm parser~~ [LiquidDoc] Parser support for optional parameters Jan 24, 2025

jamesmengo force-pushed the jm/parse_optional_params branch from 04f5d07 to 9b8824b Compare January 24, 2025 22:36

jamesmengo marked this pull request as ready for review January 24, 2025 22:53

jamesmengo requested a review from a team as a code owner January 24, 2025 22:53

jamesmengo marked this pull request as draft January 24, 2025 22:53

jamesmengo force-pushed the jm/parse_optional_params branch 2 times, most recently from c0e6ed0 to 720b222 Compare January 25, 2025 00:17

jamesmengo changed the base branch from main to jm/prevent_hover_nonexistent_snippets January 25, 2025 00:17

This was referenced Jan 25, 2025

[LiquidDoc] Prevent rendering of hover tooltip for snippets with no renderable content #729

Merged

[LiquidDoc] Formatting support for optional @param delimiters [] #734

Merged

[LiquidDoc] Hover: Wrap optional parameter names in [] #735

Merged

jamesmengo force-pushed the jm/prevent_hover_nonexistent_snippets branch 2 times, most recently from 6414426 to 9560ce1 Compare January 27, 2025 17:15

jamesmengo force-pushed the jm/parse_optional_params branch from 720b222 to 31d1bd9 Compare January 27, 2025 17:15

jamesmengo force-pushed the jm/prevent_hover_nonexistent_snippets branch from 9560ce1 to 6f1ddbb Compare January 27, 2025 17:53

jamesmengo changed the base branch from jm/prevent_hover_nonexistent_snippets to graphite-base/733 January 27, 2025 17:53

jamesmengo force-pushed the graphite-base/733 branch from 9560ce1 to f28e72d Compare January 27, 2025 17:54

jamesmengo force-pushed the jm/parse_optional_params branch from 31d1bd9 to a62068b Compare January 27, 2025 17:54

jamesmengo changed the base branch from graphite-base/733 to main January 27, 2025 17:54

jamesmengo marked this pull request as ready for review January 27, 2025 17:54

jamesmengo force-pushed the jm/parse_optional_params branch from a62068b to 5426ddb Compare January 27, 2025 17:54

jamesmengo marked this pull request as draft January 27, 2025 17:54

jamesmengo commented Jan 27, 2025

View reviewed changes

packages/liquid-html-parser/grammar/liquid-html.ohm Outdated Show resolved Hide resolved

jamesmengo commented Jan 27, 2025

View reviewed changes

packages/liquid-html-parser/src/stage-1-cst.spec.ts Outdated Show resolved Hide resolved

jamesmengo commented Jan 27, 2025

View reviewed changes

packages/liquid-html-parser/src/stage-1-cst.ts Outdated Show resolved Hide resolved

jamesmengo marked this pull request as ready for review January 27, 2025 18:04

jamesmengo added the #gsd:44310 LiquidDoc label Jan 27, 2025 — with Graphite App

jamesmengo force-pushed the jm/parse_optional_params branch from 5426ddb to dcb46c0 Compare January 27, 2025 20:36

graygilmore previously approved these changes Jan 27, 2025

View reviewed changes

packages/liquid-html-parser/grammar/liquid-html.ohm Outdated Show resolved Hide resolved

packages/liquid-html-parser/src/stage-1-cst.spec.ts Outdated Show resolved Hide resolved

packages/liquid-html-parser/src/stage-1-cst.ts Outdated Show resolved Hide resolved

jamesmengo force-pushed the jm/parse_optional_params branch 2 times, most recently from e91db5d to 374d89f Compare January 28, 2025 01:45

jamesmengo requested review from karreiro, graygilmore and EvilGenius13 January 28, 2025 02:29

jamesmengo self-assigned this Jan 28, 2025

jamesmengo requested a review from aswamy January 28, 2025 02:40

graygilmore approved these changes Jan 28, 2025

View reviewed changes

.changeset/smart-onions-pump.md Outdated Show resolved Hide resolved

packages/liquid-html-parser/grammar/liquid-html.ohm Outdated Show resolved Hide resolved

jamesmengo marked this pull request as draft January 28, 2025 21:19

jamesmengo closed this Feb 4, 2025

jamesmengo reopened this Feb 4, 2025

jamesmengo force-pushed the jm/parse_optional_params branch 2 times, most recently from ada22c8 to 26abb91 Compare February 4, 2025 20:12

jamesmengo marked this pull request as ready for review February 4, 2025 20:13

jamesmengo force-pushed the jm/parse_optional_params branch from 26abb91 to c1106c6 Compare February 4, 2025 20:16

jamesmengo marked this pull request as draft February 4, 2025 20:16

jamesmengo force-pushed the jm/parse_optional_params branch 4 times, most recently from a963b38 to c67d319 Compare February 4, 2025 20:24

jamesmengo marked this pull request as ready for review February 4, 2025 20:31

jamesmengo force-pushed the jm/parse_optional_params branch from c67d319 to 2e69f9d Compare February 4, 2025 20:35

jamesmengo commented Feb 4, 2025

View reviewed changes

Parsing support for optional param delimiters []

8c9f5bc

---- - Added a parameter `required` to the LiquidDocParamNode - Parameters with `[]` around the name will return `required: true` - Parameters with incomplete delimiters `e.g. ([missingTail)` will map to a `TextNode`

jamesmengo force-pushed the jm/parse_optional_params branch from 2e69f9d to 8c9f5bc Compare February 4, 2025 20:44

aswamy approved these changes Feb 4, 2025

View reviewed changes

EvilGenius13 approved these changes Feb 4, 2025

View reviewed changes

jamesmengo merged commit d5e8ec9 into main Feb 4, 2025
7 checks passed

jamesmengo deleted the jm/parse_optional_params branch February 4, 2025 21:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[LiquidDoc] Parser support for optional parameters #733

[LiquidDoc] Parser support for optional parameters #733

jamesmengo commented Jan 24, 2025 •

edited

Loading

jamesmengo commented Jan 24, 2025 •

edited

Loading

graygilmore left a comment

jamesmengo commented Feb 4, 2025

jamesmengo Feb 4, 2025

aswamy left a comment

[LiquidDoc] Parser support for optional parameters #733

[LiquidDoc] Parser support for optional parameters #733

Conversation

jamesmengo commented Jan 24, 2025 • edited Loading

What are you adding in this PR?

What's next? Any followup issues?

Before you deploy

jamesmengo commented Jan 24, 2025 • edited Loading

graygilmore left a comment

Choose a reason for hiding this comment

jamesmengo commented Feb 4, 2025

jamesmengo Feb 4, 2025

Choose a reason for hiding this comment

aswamy left a comment

Choose a reason for hiding this comment

jamesmengo commented Jan 24, 2025 •

edited

Loading

jamesmengo commented Jan 24, 2025 •

edited

Loading