Skip to content
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

V4 proposed non-breaking space token #3909

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dbolack-ab
Copy link
Collaborator

@dbolack-ab dbolack-ab commented Nov 23, 2024

Description

Implements :> for a series of non-breaking spaces analogous to : based hard breaks per V4 proposed discussion.

This serves to evaluate the utility of this feature.

Needs tests.

Related Issues or Discussions

Relates to discussion #2879

QA Instructions, Screenshots, Recordings

Implements as an inline token. Use : followed by one > for each non-breaking space.

Reviewer Checklist

Please replace the list below with specific features you want reviewers to look at.

*Reviewers, refer to this list when testing features, or suggest new items *

  • Verify new features are functional
    • Test any number of > after the :
  • Verify old features have not broken
    • Verify it doesn't get parsed in code blocks.
  • Test for edge cases / try to break things
    • ??
  • Identify opportunities for simplification and refactoring
  • Check for code legibility and appropriate comments

@dbolack-ab dbolack-ab added 🔍 R0 - Needs first review 👀 PR ready but has not been reviewed 🚀 V4 Wait for V4 experiment labels Nov 23, 2024
@calculuschild
Copy link
Member

calculuschild commented Nov 23, 2024

As long as these syntaxes don't conflict with existing documents, I think this one should be pretty safe. The implementation looks good at first glance as well.

What remains would be to double-check edge cases with other syntaxes that use : and define what the result should be in that case. Then add tests for those so we know we are getting the desired results.

For example, :> probably shouldn't open the emoji auto-suggest (I don't think it does, so good).

Also potential conflicts with multiline and single line term : definition blocks. What should the following do?

Term ::> Definition

Etc.

@dbolack-ab
Copy link
Collaborator Author

As long as these syntaxes don't conflict with existing documents, I think this one should be pretty safe. The implementation looks good at first glance as well.

That's largely why I let this loose without tests. I don't seem to be particularly good at guessing how the userbase will break the code. :)

For example, :> probably shouldn't open the emoji auto-suggest (I don't think it does, so good).

I didn't spot that happening when I tested, but I could have overlooked it.

Also potential conflicts with multiline and single line term : definition blocks. What should the following do?

I think that can be avoided with a slightly smarter regex to ensure that ::> isn't matched.

@calculuschild
Copy link
Member

I think that can be avoided with a slightly smarter regex to ensure that ::> isn't matched.

Probably, yes. And we should still add a test so we can lock down what we want the output to be in that case.

@dbolack-ab
Copy link
Collaborator Author

I think that can be avoided with a slightly smarter regex to ensure that ::> isn't matched.

Probably, yes. And we should still add a test so we can lock down what we want the output to be in that case.

In the mean time I will sit on this to see if there are any other concerns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experiment 🔍 R0 - Needs first review 👀 PR ready but has not been reviewed 🚀 V4 Wait for V4
Projects
Status: Backlog
Development

Successfully merging this pull request may close these issues.

3 participants