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

Added addAlias method for tokens #1524

Closed
wants to merge 3 commits into from

Conversation

RunDevelopment
Copy link
Member

This PR adds an addAlias method to the Token class.

The new method appends the given arguments to the existing aliases of a token. Nothing will be returned.
This behavior is similar to Array.prototype.push.

Examples:

let t = new Token(type, content, "alias", ...etc);
console.log(t.alias); // "alias"
t.addAlias("newAlias");
t.addAlias("alias 1", "alias 2");
console.log(t.alias); // ["alias", "newAlias", "alias 1", "alias 2"]

addAlias will always change the data representation of alias to an array.

let t = new Token(type, content);
console.log(t.alias); // undefined
t.addAlias();
console.log(t.alias); // []

@mAAdhaTTah
Copy link
Member

This might be more interesting / useful if you demonstrate where this could be used. Can you update the code this would simplify?

@RunDevelopment
Copy link
Member Author

I haven't found any language or plugin which currently has the need to add aliases to tokens, so there is no code to be simplified right now.

However, #1519 has to do exactly that and it can be compressed into a single if-statement using addAlias:

// class-name alias to tags which refer to classes
if (token.type === 'tag' && token.content.length === 2 && token.content[0].type === 'punctuation'
    && typeof token.content[1] === 'string' && /^[A-Z]/.test(token.content[1]))
	token.addAlias('class-name');

I added addAlias because after writing #1519, I wrote another hook and had to implement it again. This PR is for eliminating this redundancy.

Plus, I think that something as basic as adding an alias to a token should be simple.

@mAAdhaTTah
Copy link
Member

Do we still need this? If there's no current code that needs it, I'd prefer not to land it until it has a use-case.

@RunDevelopment
Copy link
Member Author

Well, your right.

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.

2 participants