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

Document standard tokens and provide examples #3104

Merged
merged 20 commits into from Oct 21, 2021

Conversation

hoonweiting
Copy link
Contributor

This PR creates a new page on the website called tokens.html, where the standard token names are listed in a definition list, with at least one example provided for each definition. I'm only familiar with like, maybe ten languages at best, so I would really appreciate anyone's and everyone's feedback on the definitions! Also, more ideas for examples are very welcome!!

I also have a few questions in particular:

  1. Should inserted and deleted be collapsed into one entry? This will also allow us to use a combined example to showcase insertions and deletions, which seems more legit, lol. Also, should Brainfuck be showcased as an example of using inserted and deleted?
  2. Should attr-name and attr-value be collapsed into one entry as well?
  3. In particular, I would like feedback on symbol, namespace, prolog, and cdata, because I honestly have no idea what they are, and copied them off google, haha.

And one more thing, I could have sworn I saw in a comment somewhere about "aliasing semantics" when it comes to token names. But for some reason I can't seem to find it right now. I do want to include something along those lines in the introductory blurb to encourage contributors to alias non-standard tokens with semantically-similar standard tokens! Just really not sure how to phrase it.

This hopes to resolve the first half of #2849.

@github-actions
Copy link

github-actions bot commented Oct 3, 2021

No JS Changes

Generated by 🚫 dangerJS against a27a11c

Copy link
Member

@RunDevelopment RunDevelopment left a comment

Choose a reason for hiding this comment

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

Great work @hoonweiting!

Should inserted and deleted be collapsed into one entry?
Should attr-name and attr-value be collapsed into one entry as well?

I think that's a good idea for both.

In particular, I would like feedback on symbol, namespace, prolog, and cdata

You got all of them right. Just one thing needs to be added to namespace (see comment).

I could have sworn I saw in a comment somewhere about "aliasing semantics"

Aliasing semantics work like this: If you have a token that is more specific, you can use a custom token name and give it a standard token name as an alias. E.g. (take from Rust):

'function-definition': {
	pattern: /(\bfn\s+)\w+/,
	lookbehind: true,
	alias: 'function'
},

Some languages also abuse aliasing to get a particular highlighting color but that shouldn't be done.

tokens.html Outdated Show resolved Hide resolved
tokens.html Outdated Show resolved Hide resolved
tokens.html Outdated Show resolved Hide resolved
tokens.html Outdated Show resolved Hide resolved
tokens.html Outdated Show resolved Hide resolved
tokens.html Outdated Show resolved Hide resolved
tokens.html Outdated Show resolved Hide resolved
tokens.html Outdated Show resolved Hide resolved
tokens.html Outdated Show resolved Hide resolved
tokens.html Outdated Show resolved Hide resolved
@hoonweiting
Copy link
Contributor Author

Alright, most of the changes have been made, just need to add more examples, particularly for constant.


Also I have three examples for inserted and deleted now (all taken from the examples page), but I have no idea why there's no vertical margin between them (unlike other tokens with multiple examples). Upon opening dev tools, I see that the style is being applied to the <div class="code-toolbar"> tag directly, so I'm seeing <div class="code-toolbar" style="margin: 0px;"> for all the diff/git codeblocks, compared to <div class="code-toolbar" style="margin: 8px 0px;"> in the non-last url, property, and important (basically the other tokens with multiple examples) codeblocks.

On further testing it seems that it's a problem of having two codeblocks with the same language next to each other, but this is not a perfect explanation either (css with css, and markup with markup is fine, but diff/markdown/python are not, of those I just tried).

And the above two paragraphs only hold true in Chrome (and Edge). In Firefox, none of them have vertical spacing. I don't know what's going on tbh, lol. Commenting out the Toolbar and Show Language plugins seemed to return the margins consistently, so I'll guess the problem lies somewhere there.

However, not sure if all this will be a different story when published online, since not all elements are loading for me locally (like the Prism logo with that download button). I doubt it, but I can't say for certain.

I thought this was gonna be a short paragraph but I guess not! Maybe I should open a new issue for this.

@RunDevelopment
Copy link
Member

Also I have three examples for inserted and deleted now

Sounds a bit much. Maybe we could just use the middle example? A git diff should be understood by most people.

since not all elements are loading for me locally (like the Prism logo with that download button)

npm start opens a small local server for the website.

Maybe I should open a new issue for this.

Please do. That sounds like a general problem with Toolbar.

@hoonweiting
Copy link
Contributor Author

Maybe we could just use the middle example?

Sure thing!

npm start opens a small local server for the website.

Oh damn, I didn't know there was something like this. This is fantastic, thank you!

That sounds like a general problem with Toolbar.

Okay, I will do so after more testing. Inb4 it's me forgetting to escape some tags or making some other typo somewhere, haha.

@hoonweiting
Copy link
Contributor Author

Also if anyone is interested, here's the comment on "aliasing semantics" that I finally managed to find: #354 (comment)

@hoonweiting
Copy link
Contributor Author

I don't know if I'm completely happy with this, the examples feel pretty inconsistent to me, some are foobar types, some are very toy examples, others are more legit. But that's honestly probably not so important in the grand scheme of things, so maybe it's much ado about nothing.

I do think the introductory blurb needs work though. I'm not sure if it sounds a little repetitive, or if it's unclear.

tokens.html Outdated Show resolved Hide resolved
tokens.html Outdated Show resolved Hide resolved
tokens.html Outdated Show resolved Hide resolved
tokens.html Outdated Show resolved Hide resolved
tokens.html Outdated Show resolved Hide resolved
@RunDevelopment
Copy link
Member

With #3110 being resolved, I think this is ready. What do you think @hoonweiting?

@hoonweiting
Copy link
Contributor Author

@RunDevelopment Hello! Sorry for the delay, had a bunch of other things on my plate. Yeah I think the doc is almost there, just remembered I should probably link to the page from other pages, else no one's gonna find it, haha. After that, I'll mark the PR as ready!

@hoonweiting
Copy link
Contributor Author

I'm low-key having some trouble with this, haha. I don't know if the page needs a link in the footer. I'd go with yes, but not every page is linked in the footer, so I don't know what I should do. (Certainly, I'd like to change that (add all pages to the footer), but maybe not as part of this PR.) Other than that, I'll link it in extending.html and faq.html, but I don't know if it flows well, or is too obscure like the examples page (which is also not linked in the footer).

Also, I noticed some inline code wasn't being highlighted on the FAQ page, so I added the language-none class to the body tag. And I just noticed that most other pages split the page text into multiple lines. I didn't do that for tokens.html though, whoops.

Also hmm, not sure if I should have pulled and merged from the master branch. Maybe I shouldn't have? I have no idea, I don't have a lot of experience with squashed PR merges, but I agree it's way neater than the normal merge.

@hoonweiting hoonweiting marked this pull request as ready for review October 21, 2021 17:23
@RunDevelopment
Copy link
Member

RunDevelopment commented Oct 21, 2021

You link to the tokens page in the most relevant sections, so I think that's enough. We can always add more links in the future.

And I just noticed that most other pages split the page text into multiple lines.

Don't worry about it. We aren't consistent with that at all... Some pages just have one long line per paragraph, others split it into sentences, and again others wrap at ~120 chars.

Yeah, I'll make a PR for that after merging this. Edit: Prettier's HTML formatting doesn't look good. Let's just keep it as is for now.

Also hmm, not sure if I should have pulled and merged from the master branch. Maybe I shouldn't have?

It's totally fine. Since everything is squashed into one commit you can have as many commits as you like, be it normal commits or merge commits.

@RunDevelopment RunDevelopment merged commit 3755120 into PrismJS:master Oct 21, 2021
@RunDevelopment
Copy link
Member

Thank you very much for contributing @hoonweiting!

@hoonweiting
Copy link
Contributor Author

Ah right, guess my brain isn't working! I was kinda spooked when I saw that I had like 60ish commits waiting to be pushed to my branch after merging, and was afraid that there'll be a wall of commits. Guess that wasn't the case after all!

Thank you @RunDevelopment for being super supportive and for doing so much 😄

@hoonweiting hoonweiting deleted the 2849-token-type-docs branch October 21, 2021 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants