Skip to content

docs: mention usage of regexp literal in text expressions #5640

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

Merged
merged 3 commits into from
Jul 10, 2021
Merged

docs: mention usage of regexp literal in text expressions #5640

merged 3 commits into from
Jul 10, 2021

Conversation

ignatiusmb
Copy link
Member

Closes #5318

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with npm test and lint the project with npm run lint

@Conduitry
Copy link
Member

Featuring this alongside other much more important and generally useful documentation still feels odd to me. As I mentioned in the original issue, I don't really have an answer where this should appear. I also don't know how many times this has come up in practice, or whether the original issue was @tanhauhau specifically looking for bugs.

@tanhauhau
Copy link
Member

i raised the issue as a bug, but i'm not sure whether it should be documented as an intended behavior rather than fixing it

@Conduitry
Copy link
Member

When this came up in the maintainers chat - https://discord.com/channels/457912077277855764/571775594002513921/749639227444297759 - pngwn and I talked about how it seems there's not really a reasonable way to handle this. If someone does {/if}/} then we can't tell whether this is a closing if tag or a regex.

@stale
Copy link

stale bot commented Jun 26, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale-bot label Jun 26, 2021
@pngwn pngwn added site and removed site: docs labels Jun 26, 2021
@stale stale bot removed stale-bot labels Jun 26, 2021
@pngwn pngwn added documentation and removed site labels Jun 27, 2021
Copy link
Member

@benmccann benmccann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem like the worst place in the docs to put it, but I'd probably put it in an aside box instead of the main text

@@ -141,6 +141,13 @@ Text can also contain JavaScript expressions:
<p>{a} + {b} = {a + b}.</p>
```

---

However, if you're using a regular expression (`RegExp`) [literal notation](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp#literal_notation_and_constructor), you'll need to wrap it in parentheses.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
However, if you're using a regular expression (`RegExp`) [literal notation](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp#literal_notation_and_constructor), you'll need to wrap it in parentheses.
> If you're using a regular expression (`RegExp`) [literal notation](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp#literal_notation_and_constructor), you'll need to wrap it in parentheses.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Placing it in an aside box seems nice. I haven't seen what it'll look like on the page if we do that, but I see in the current docs that blockquotes are always placed after a main text first. Should we also remove the separator above? What about the example code after it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This blue box is an example: https://svelte.dev/docs#get

Yes, I think we could remove the separator in that case. Can we put the code example in the box? (maybe if you prefix each line with > it will do that. you can test by running the site locally, which I think is cd site; npm install; npm run update; npm run dev)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks a bit weird

image

@ignatiusmb
Copy link
Member Author

I've changed and merged it to the previous row, I think this is clearer and follows the style (like https://svelte.dev/docs#get) better

image

@benmccann benmccann merged commit 5334f4a into sveltejs:master Jul 10, 2021
@ignatiusmb ignatiusmb deleted the docs/forward-slash-expression branch July 10, 2021 04:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to start an expression with '/'
5 participants