fix: allow ] in filename code block #2169

Barbapapazes · 2023-07-12T16:17:04Z

🔗 Linked issue

❓ Type of change

📖 Documentation (updates to the documentation or readme)
🐞 Bug fix (a non-breaking change that fixes an issue)
👌 Enhancement (improving an existing functionality like performance)
✨ New feature (a non-breaking change that adds functionality)
⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

fix #2166

For a code block, if filename was [...page].vue, regex was stoping at [...page. This PR fix it.

📝 Checklist

I have linked an issue or discussion.
I have updated the documentation accordingly.

netlify · 2023-07-12T16:17:11Z

✅ Deploy Preview for nuxt-content canceled.

Name	Link
🔨 Latest commit	`e0ff29c`
🔍 Latest deploy log	https://app.netlify.com/sites/nuxt-content/deploys/64be72478514bf000792733f

nobkd · 2023-07-12T16:25:49Z

src/runtime/markdown-parser/handler/utils.ts

@@ -20,7 +20,7 @@ export function parseThematicBlock (lang: string) {

  const language = lang.replace(/[{|[](.+)/, '').match(/^[^ \t]+(?=[ \t]|$)/)
  const highlightTokens = lang.match(/{([^}]*)}/)
-  const filenameTokens = lang.match(/\[([^\]]*)\]/)
+  const filenameTokens = lang.match(/\[(.*)\]/) /** Allow ']' in filename. @see https://regex101.com/r/Hm0t5m/1 */


I think we used [^\]] to allow using lists in the meta part.
Here's an example what would not work anymore if we match the longest string.

```language [filename]{hightlights} meta=[...]

It would match this content filename]{hightlights} meta=[... as the filename

I thought we could use something like \] in the markdown and prevent matching those somehow

Oooh I understand. I do it too quickly!

I think, that this might work: /\[((\\]|[^\]])*)\]/

This part (\\]|[^\]])* matches zero or one or a sequence of: \] or characters that are not ]

Only if our string has no more ] after a \] it terminates the match at a \].

So this works:

```ts [[...slug\].vue] meta=[]

=> [...slug\].vue as the first group match.
Then we only have to replace \] which in reality is \\] with ]

Yes but if there is no meta, this regex is not working.

https://regex101.com/r/bG0Net/1

EDIT: User must set a , I understand.

nobkd

This should be all the necessary changes, but we'll see...
This is currently not really tested.

I tried both \\] and \] and it seems that we need the first variant with two backslashes to get one real backslash from the Markdown.

So, this works in the playground:

```ts [[...slug\\].vue] ...

nobkd · 2023-07-12T21:14:07Z

src/runtime/markdown-parser/handler/utils.ts

@@ -20,7 +20,7 @@ export function parseThematicBlock (lang: string) {

  const language = lang.replace(/[{|[](.+)/, '').match(/^[^ \t]+(?=[ \t]|$)/)
  const highlightTokens = lang.match(/{([^}]*)}/)
-  const filenameTokens = lang.match(/\[([^\]]*)\]/)
+  const filenameTokens = lang.match(/\[(.*)\]/) /** Allow ']' in filename. @see https://regex101.com/r/Hm0t5m/1 */


Suggested change

const filenameTokens = lang.match(/\[(.*)\]/) /** Allow ']' in filename. @see https://regex101.com/r/Hm0t5m/1 */

const filenameTokens = lang.match(/\[((\\]|[^\]])*)\]/)

And because of GitHub, I'm not allowed to suggest changes out of some range:

Change for L29:

- filename: Array.isArray(filenameTokens) && filenameTokens[1] ? filenameTokens[1] : undefined, + filename: Array.isArray(filenameTokens) && filenameTokens[1] ? filenameTokens[1].replace(/\\]/g, ']') : undefined,

To remove the now redundant backslashes.

nobkd · 2023-07-12T21:19:45Z

src/runtime/markdown-parser/handler/utils.ts

@@ -20,7 +20,7 @@ export function parseThematicBlock (lang: string) {

  const language = lang.replace(/[{|[](.+)/, '').match(/^[^ \t]+(?=[ \t]|$)/)
  const highlightTokens = lang.match(/{([^}]*)}/)
-  const filenameTokens = lang.match(/\[([^\]]*)\]/)
+  const filenameTokens = lang.match(/\[(.*)\]/) /** Allow ']' in filename. @see https://regex101.com/r/Hm0t5m/1 */
  const meta = lang.replace(/^\w*\s*(\[[^\]]*\]|\{[^}]*\})?\s*(\[[^\]]*\]|\{[^}]*\})?\s*/, '')


Suggested change

const meta = lang.replace(/^\w*\s*(\[[^\]]*\]|\{[^}]*\})?\s*(\[[^\]]*\]|\{[^}]*\})?\s*/, '')

const meta = lang.replace(/^\w*\s*(\[((\\]|[^\]])*)\]|\{[^}]*\})?\s*(\[((\\]|[^\]])*)\]|\{[^}]*\})?\s*/, '')

To replace the correct part, to have the meta left.

I refactor this to be more understandable and to reuse previous regex.

Barbapapazes · 2023-07-13T08:36:56Z

I will also update documentation to add to warning about usage.

nuxt-studio · 2023-07-13T08:54:53Z

✅ Live Preview ready!

Name	Edit	Preview	Latest Commit
content	Edit on Studio ↗︎	View Live Preview	`e0ff29c`

nuxt-studio · 2023-07-13T08:54:56Z

✅ Live Preview ready!

Name	Edit	Preview	Latest Commit
content	Edit on Studio ↗︎	View Live Preview	`3a69417`

nobkd · 2023-07-13T10:31:22Z

src/runtime/markdown-parser/handler/utils.ts

-  const meta = lang.replace(/^\w*\s*(\[[^\]]*\]|\{[^}]*\})?\s*(\[[^\]]*\]|\{[^}]*\})?\s*/, '')
+  const languageMatches = lang.replace(/[{|[](.+)/, '').match(/^[^ \t]+(?=[ \t]|$)/)
+  const highlightTokensMatches = lang.match(/{([^}]*)}/)
+  const filenameMatches = lang.match(/\[((\S)*)\]/) /** Allow ']' in filename. */


I can see that you don't want an escape character to have ] in your filename, but we didn't want to depend on spaces. If we now change this, it could be a breaking change.

My variant from before would introduce an escape character (e.g. \\) for ] but

would not need spaces after the last ] that belongs to the filename

allow spaces in the filename.

You could not do this anymore:

```ts [filename]meta=[]

or

```ts [filename/title with spaces]

Spaces in the filename for example are already used in the nuxt/nuxt docs. Some examples:

https://github.com/nuxt/nuxt/blob/350788419c414317354de9900cd049fa750456cc/docs/7.migration/4.meta.md?plain=1#L27

https://github.com/nuxt/nuxt/blob/350788419c414317354de9900cd049fa750456cc/docs/1.getting-started/5.routing.md?plain=1#L17

https://github.com/nuxt/nuxt/blob/350788419c414317354de9900cd049fa750456cc/docs/1.getting-started/5.routing.md?plain=1#L25

#2067 was the last change to this regexes, there should also be some ideas.

you've convinced me. I will update regex to allow esacape ] and update docs in that ways (documenting why we choose this method).

src/runtime/markdown-parser/handler/utils.ts

nobkd · 2023-07-13T11:01:12Z

src/runtime/markdown-parser/handler/utils.ts

+  const meta = lang
+    .replace(languageMatches?.[0] ?? '', '')
+    .replace(highlightTokensMatches?.[0] ?? '', '')
+    .replace(filenameMatches?.[0] ?? '', '')
+    .trim()


Thanks for this, it makes this part so much more readable! ❤️

Co-authored-by: nobkd <44443899+nobkd@users.noreply.github.com>

Barbapapazes · 2023-07-15T22:01:47Z

@nobkd we have an issue because a JS string is automatically escape \ in string. So [[...page\].vue] is resolved as [[...page].vue]

Barbapapazes · 2023-07-15T22:05:36Z

We will need to use a double \. I'll update PR with this usage.

nobkd · 2023-07-15T22:08:54Z

See this #2169 (review)

We need to use double escaping: \\]

There's probably a better escape character, but \ is the generally used one, so I went with it. Do you have a better idea how to escape?

nobkd · 2023-07-15T22:09:44Z

We will need to use a double \. I'll update PR with this usage.

Exactly

Barbapapazes · 2023-07-15T22:12:24Z

See this #2169 (review)

We need to use double escaping: \\]

There's probably a better escape character, but \ is the generally used one, so I went with it. Do you have a better idea how to escape?

Oh yes, I've not seen this comment! Thanks

playground/basic/components/content/ProseCode.vue

Co-authored-by: nobkd <44443899+nobkd@users.noreply.github.com>

playground/basic/components/content/ProseCode.vue

farnabaz

Thanks guys.
You're awesome ❤️

nobkd · 2023-07-30T01:29:35Z

Just to have this in one place, I think this bug was previously introduced with #2067

https://github.com/nuxt/content/pull/2067/files#diff-fd60d817a96ebccaa6b1456127b1e4589b3e10487ddb84362ab6e41ab1ede143L22-R23

I think, there was decided, that we could allow [] and {} in the meta part, and therefore changed the regex...

fix: allow ] in filename

d8e8c93

Barbapapazes changed the title ~~fix: allow ] in filename~~ fix: allow ] in filename code block Jul 12, 2023

nobkd reviewed Jul 12, 2023

View reviewed changes

nobkd suggested changes Jul 12, 2023

View reviewed changes

fix: regex parsing in code block

3a69417

Barbapapazes added 2 commits July 13, 2023 11:05

fix: '' to undefined

07a4296

fix: parsing to return undefined

8d110cb

Barbapapazes requested a review from nobkd July 13, 2023 09:18

nobkd suggested changes Jul 13, 2023

View reviewed changes

Barbapapazes marked this pull request as draft July 13, 2023 10:55

nobkd suggested changes Jul 13, 2023

View reviewed changes

src/runtime/markdown-parser/handler/utils.ts Outdated Show resolved Hide resolved

src/runtime/markdown-parser/handler/utils.ts Show resolved Hide resolved

nobkd reviewed Jul 13, 2023

View reviewed changes

Barbapapazes and others added 2 commits July 15, 2023 15:12

Update src/runtime/markdown-parser/handler/utils.ts

28707f5

Co-authored-by: nobkd <44443899+nobkd@users.noreply.github.com>

Update src/runtime/markdown-parser/handler/utils.ts

aae3254

Co-authored-by: nobkd <44443899+nobkd@users.noreply.github.com>

fix backslash issue

a9b8afb

Barbapapazes requested a review from nobkd July 15, 2023 22:11

Barbapapazes added 3 commits July 16, 2023 00:17

test: fix

bd4fcfa

test: fix

3d8b2cb

test: remove

c51b08e

Barbapapazes marked this pull request as ready for review July 15, 2023 22:32

This comment was marked as resolved.

Sign in to view

farnabaz reviewed Jul 24, 2023

View reviewed changes

playground/basic/components/content/ProseCode.vue Outdated Show resolved Hide resolved

Apply suggestions from code review

a536c58

Co-authored-by: nobkd <44443899+nobkd@users.noreply.github.com>

farnabaz reviewed Jul 24, 2023

View reviewed changes

playground/basic/components/content/ProseCode.vue Outdated Show resolved Hide resolved

Update playground/basic/components/content/ProseCode.vue

e0ff29c

farnabaz approved these changes Jul 24, 2023

View reviewed changes

farnabaz merged commit 7a6ab10 into nuxt:main Jul 24, 2023
7 checks passed

farnabaz mentioned this pull request Jul 25, 2023

v2.7.2 #2194

Merged

This was referenced Jul 29, 2023

docs: escape ] in code-block filenames nuxt/nuxt#22389

Merged

docs: escape ] in code-block filename nuxt-modules/i18n#2275

Merged

nobkd mentioned this pull request Jul 30, 2023

escaped ] not working outside the playground #2201

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: allow ] in filename code block #2169

fix: allow ] in filename code block #2169

Barbapapazes commented Jul 12, 2023 •

edited

netlify bot commented Jul 12, 2023 •

edited

nobkd Jul 12, 2023 •

edited

nobkd Jul 12, 2023 •

edited

Barbapapazes Jul 12, 2023

nobkd Jul 12, 2023 •

edited

Barbapapazes Jul 13, 2023 •

edited

nobkd left a comment •

edited

nobkd Jul 12, 2023

nobkd Jul 12, 2023 •

edited

nobkd Jul 12, 2023 •

edited

Barbapapazes Jul 13, 2023 •

edited

Barbapapazes commented Jul 13, 2023

nuxt-studio bot commented Jul 13, 2023 •

edited

nuxt-studio bot commented Jul 13, 2023

nobkd Jul 13, 2023

Barbapapazes Jul 13, 2023

nobkd Jul 13, 2023

Barbapapazes commented Jul 15, 2023

Barbapapazes commented Jul 15, 2023

nobkd commented Jul 15, 2023

nobkd commented Jul 15, 2023

Barbapapazes commented Jul 15, 2023

This comment was marked as resolved.

farnabaz left a comment

nobkd commented Jul 30, 2023 •

edited

	const filenameTokens = lang.match(/\[(.)\]/) /* Allow ']' in filename. @see https://regex101.com/r/Hm0t5m/1 */
	const filenameTokens = lang.match(/\[((\\]\|[^\]])*)\]/)

	const meta = lang.replace(/^\w\s(\[[^\]]\]\|\{[^}]\})?\s(\[[^\]]\]\|\{[^}]\})?\s/, '')
	const meta = lang.replace(/^\w\s(\[((\\]\|[^\]]))\]\|\{[^}]\})?\s(\[((\\]\|[^\]]))\]\|\{[^}]\})?\s/, '')

fix: allow ] in filename code block #2169

fix: allow ] in filename code block #2169

Conversation

Barbapapazes commented Jul 12, 2023 • edited

🔗 Linked issue

❓ Type of change

📚 Description

📝 Checklist

netlify bot commented Jul 12, 2023 • edited

✅ Deploy Preview for nuxt-content canceled.

nobkd Jul 12, 2023 • edited

Choose a reason for hiding this comment

nobkd Jul 12, 2023 • edited

Choose a reason for hiding this comment

Barbapapazes Jul 12, 2023

Choose a reason for hiding this comment

nobkd Jul 12, 2023 • edited

Choose a reason for hiding this comment

Barbapapazes Jul 13, 2023 • edited

Choose a reason for hiding this comment

nobkd left a comment • edited

Choose a reason for hiding this comment

nobkd Jul 12, 2023

Choose a reason for hiding this comment

nobkd Jul 12, 2023 • edited

Choose a reason for hiding this comment

nobkd Jul 12, 2023 • edited

Choose a reason for hiding this comment

Barbapapazes Jul 13, 2023 • edited

Choose a reason for hiding this comment

Barbapapazes commented Jul 13, 2023

nuxt-studio bot commented Jul 13, 2023 • edited

✅ Live Preview ready!

nuxt-studio bot commented Jul 13, 2023

✅ Live Preview ready!

nobkd Jul 13, 2023

Choose a reason for hiding this comment

Barbapapazes Jul 13, 2023

Choose a reason for hiding this comment

nobkd Jul 13, 2023

Choose a reason for hiding this comment

Barbapapazes commented Jul 15, 2023

Barbapapazes commented Jul 15, 2023

nobkd commented Jul 15, 2023

nobkd commented Jul 15, 2023

Barbapapazes commented Jul 15, 2023

This comment was marked as resolved.

farnabaz left a comment

Choose a reason for hiding this comment

nobkd commented Jul 30, 2023 • edited

Barbapapazes commented Jul 12, 2023 •

edited

netlify bot commented Jul 12, 2023 •

edited

nobkd Jul 12, 2023 •

edited

nobkd Jul 12, 2023 •

edited

nobkd Jul 12, 2023 •

edited

Barbapapazes Jul 13, 2023 •

edited

nobkd left a comment •

edited

nobkd Jul 12, 2023 •

edited

nobkd Jul 12, 2023 •

edited

Barbapapazes Jul 13, 2023 •

edited

nuxt-studio bot commented Jul 13, 2023 •

edited

nobkd commented Jul 30, 2023 •

edited