-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Revert "Highlight all EBNF snippets. Add missing semicolons." #14958
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
Conversation
This reverts commit a60c4e7.
Can we not revert the PR and instead replace semicolons with some other less misleading chars? I thought about dots or maybe some other unicode characters? |
But why? Scala does not require termination characters, so why add one for EBNF? That is super misleading. Besides, the style without termination character has been used for every Scala grammar since the start and is also the style of the official Java grammar. So, let's not mess with this! |
The need of having some termination character is related only to the fact that we use highlight.js as a highlighting tool and the rules of highlighting EBNF are then much easier. We can also revert this and have small styling issues in ebnf snippets which we can solve later in different way. |
@pikinier20 Yes, let's do this. Never let your tooling dictate your documentation content. |
I'm gonna make a PR from this branch to stable documentation branch to have it updated there. Then we'll have a backport PR that will also change it on main branch. First, we probably need to merge this #14994 |
I see that this was done in the past in https://github.com/lampepfl/dotty/pull/14958/files, but then reverted in scala#14958. Like many commits, there really isn't an explanation of the revert, but from reading between the lines I assume the `;` was the actual issue, not the syntax highlighting. As it was pointed out, syntax.js doesn't actually support `ebnf`. They do say they support `bnf`, but that didn't really work when I was testing. Either way, this pr makes sure that we _do_ mark the snippets as `ebnf`. The reason for this isn't necessarily so that we _get_ syntax highlighting for these, but so that syntax.js doesn't infer the wrong type of syntax and provide odd highlighting like we currently have. This also helps to ensure screen readers know what type of codeblock this is. fixes scala#14697
I see that this was done in the past in https://github.com/lampepfl/dotty/pull/14958/files, but then reverted in #14958. Like many commits, there really isn't an explanation of the revert, but from reading between the lines I assume the `;` was the actual issue, not the syntax highlighting. As it was pointed out, syntax.js doesn't actually support `ebnf`. They do say they support `bnf`, but that didn't really work when I was testing. Either way, this pr makes sure that we _do_ mark the snippets as `ebnf`. The reason for this isn't necessarily so that we _get_ syntax highlighting for these, but so that syntax.js doesn't infer the wrong type of syntax and provide odd highlighting like we currently have. This also helps to ensure screen readers know what type of codeblock this is. fixes #14697
This reverts commit a60c4e7.