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

insertBefore and duplicate tokens #1525

Closed
RunDevelopment opened this issue Aug 15, 2018 · 1 comment · Fixed by #1628
Closed

insertBefore and duplicate tokens #1525

RunDevelopment opened this issue Aug 15, 2018 · 1 comment · Fixed by #1628

Comments

@RunDevelopment
Copy link
Member

Right now, the documentation of insertBefore does not specify how the function behaves if the grammar of inside and insert have keys in common.

Example

Let's assume the following language:

Prism.languages.foo = {
    'a': /a/,
    'b': /b/,
    'c': /c/,
    'd': /d/
};

Now, we insert a new pattern for a existing token:

Prism.languages.insertBefore('foo', 'd', {
    'a': /new/
});

// result:
{
    'a': /new/, // old position, new pattern
    'b': /b/,
    'c': /c/,
    'd': /d/
};

And in the other direction: (Prism.languages.foo as originally defined)

Prism.languages.insertBefore('foo', 'b', {
    'd': /new/
});

// result:
{
    'a': /a/,
    'd': /d/, // new position, old pattern
    'b': /b/,
    'c': /c/
};
@RunDevelopment
Copy link
Member Author

RunDevelopment commented Aug 15, 2018

I can imagine 3 solutions for this:

  1. Throw an error.
    We would detect duplicates and throw if we were to find one. Let the creator of the language decide how this case should be handled.
  2. Remove duplicates from inside.
    We remove all duplicates from the grammar of inside and then go on as usual. This ensures that new patterns are both at the correct position and not overwritten by old patterns.
  3. Don't insert duplicates.
    If we detect that a token of insert is also present in inside, we ignore it.

I personally prefer option number 2 because it's what I intuitively expected.

Thoughts?

mAAdhaTTah pushed a commit that referenced this issue Dec 1, 2018
Fix #1525 and implement option number 2.
ggrossetie pushed a commit to ggrossetie/prism that referenced this issue Mar 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants