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

Add language BSL (1C:Enterprise) #2520

Merged
merged 14 commits into from
Aug 17, 2020
Merged

Add language BSL (1C:Enterprise) #2520

merged 14 commits into from
Aug 17, 2020

Conversation

Diversus23
Copy link
Contributor

The language BSL (1C:Enterprise) added.

There is one problem, since the constructions use /u (Cyrillic language constructions are used) the "npm test" command is down.

2020-08-13_18-32-00
2020-08-13_18-33-29

@Diversus23
Copy link
Contributor Author

@RunDevelopment
Copy link
Member

First, please let me apologize. I assumed that you made a language definition for private use in which case the u
flag and ES2018 lookbehinds might not have been an issue for you. But for Prism, where we have to support older browsers as well, we can't use either. I'm sorry for giving you the wrong information.

To make the tests pass, we have to get rid of the u flag and the ES2018 lookbehinds.

Getting rid of the ES2018 lookbehinds is easy: you just have to use workaround 2.

Getting rid of the u flag is a lot harder... We basically have to create a character class that behaves like [\p{L}\p{N}_] without the use of Unicode properties. In #2519, I said to use Babel but that's not an option here because Babel will transpile \p{L} into a pattern that is 4kB long. We need \p{L} multiple times, so let's not do this.

Instead, let's redefine the problem. Since we only really need to support Cyrillic script, we can cut down on the numbers of characters. Instead of [\p{L}\p{N}_], let's use [\w\u0400-\u0484\u0487-\u052f\u1c80-\u1c88\u1d2b\u1d78\u2de0-\u2dff\ua640-\ua69f\ufe2e\ufe2f]. It will match all Latin and Cyrillic word characters, is reasonably short, and doesn't need the u flag.

@Diversus23
Copy link
Contributor Author

  1. I have a question about tests. I run the test "npm run test: languages -- --language=bsl --pretty" and the test tells me that everything is OK. But then in the global test "npm test" my tests do not pass. Why don't I also run global test blocks in a local test? It is a long time to wait for the entire global test to be checked. Is it possible to run npm test only for my language, but completely?
  2. Consider making sure that regular expressions support national languages natively in prismjs. There are languages that don't work based on English syntax https://en.wikipedia.org/wiki/Non-English-based_programming_languages and there are many of them. Or still deal with the flag /u. Prismjs will not be able to support them normally without dancing with a tambourine :)
  3. Replaced [\p{L}\p{N}_] = > [\w\u0400-\u0484\u0487-\u052f\u1c80-\u1c88\u1d2b\u1d78\u2de0-\u2dff\ua640-\ua69f\ufe2e\ufe2f] but the tests don't pass
    image
  4. There is a problem with numbers. Pattern 'number': /(?:\b\d+.?\d*|\B.\d+)(?:E[+-]?\d+)?/i does not work normally for the same reason because there is \b
    image

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.

  1. npm run test:languages only runs the language tests (the test cases under tests/languages). Unfortunately, there isn't an option to run all tests for a specific language yet.
  2. We are planning to move to ES6 eventually, so the u flag will be supported natively then. This doesn't completely solve the issue but it certainly helps.
    Supporting the u flag right now means transpiling code which can create huge patterns (e.g. 4kB for a single \p{L}). This is a problem because the relatively small size of our language definitions is one of Prism's strong points.
  3. That's because of the ES2018 lookbehind. See comments.
  4. See comments.

I also suggested a few other improvements.

components/prism-bsl.js Outdated Show resolved Hide resolved
components/prism-bsl.js Outdated Show resolved Hide resolved
components/prism-bsl.js Outdated Show resolved Hide resolved
components/prism-bsl.js Outdated Show resolved Hide resolved
components/prism-bsl.js Outdated Show resolved Hide resolved
components/prism-bsl.js Outdated Show resolved Hide resolved
components/prism-bsl.js Outdated Show resolved Hide resolved
Diversus23 and others added 6 commits August 16, 2020 16:47
Co-authored-by: Michael Schmidt <mitchi5000.ms@googlemail.com>
Co-authored-by: Michael Schmidt <mitchi5000.ms@googlemail.com>
Co-authored-by: Michael Schmidt <mitchi5000.ms@googlemail.com>
Co-authored-by: Michael Schmidt <mitchi5000.ms@googlemail.com>
Co-authored-by: Michael Schmidt <mitchi5000.ms@googlemail.com>
@Diversus23
Copy link
Contributor Author

Thank you for your help. I will be happy if my changes for the new BSL language are added to the main repository. In Russia, this language is very popular and has a huge community.

docs/Prism.html Outdated Show resolved Hide resolved
@RunDevelopment
Copy link
Member

Thanks for the changes @Diversus23!

Apart from my two comments, I think this is ready to be merged.

Please resolve the merge conflicts. This should be as simple as merging our master branch run running npm run build.

@RunDevelopment
Copy link
Member

There are still 4 files that changed:

components.js                                    | 2 +-
docs/Prism.languages.html                        | 8 +++++++-
plugins/autoloader/prism-autoloader.min.js       | 2 +-
plugins/show-language/prism-show-language.min.js | 2 +-

You also have to revert docs/Prism.languages.html.
The rest are generated minified files. In theory, you only have to run npm run build. Does that not work?

@RunDevelopment
Copy link
Member

Seems like you accidentally committed the docs/** files again. Everything else is fine.

@RunDevelopment
Copy link
Member

You can make JSDoc generate the correct line ends by converting the line ends of components/prism-core.js to LF and running npm run build again. This may be a little easier than using git to revert some files.

Btw. I filed a bug report about the CRs in the docs/** files. I'll soon make a temporary hack to fix the line ends until the bug is fixed upstream.

@Diversus23
Copy link
Contributor Author

I only ran npm run build. Changes to docs/** were created automatically. And I probably should not have put the modified docs/** files. To be honest, I don't often work with repositories with the team and I don't know what to do next to solve the problem.

@RunDevelopment
Copy link
Member

I only ran npm run build. Changes to docs/** were created automatically.

Sorry, that's a problem on our part. I will fix this soon.

Until then. to make the CI pass, do the following:

  1. Change the line ends of components/prism-core.js to LF. Most editors will have an option to do that for you.
  2. Run npm run build. The docs/** files will change (this is what we want!).
  3. Commit only the changed docs/** files. Do not commit the changes to components/prism-core.js or any other changed files outside of the docs folder.

Again, I'm really sorry that this is so difficult. This isn't something anybody should have to deal with. This will be fixed soon.

@RunDevelopment RunDevelopment merged commit 5c33f0b into PrismJS:master Aug 17, 2020
@RunDevelopment
Copy link
Member

Thank you for contributing @Diversus23!

@Diversus23
Copy link
Contributor Author

Thank you @RunDevelopment!
I am always inspired by people who do their job well and are real professionals. Good luck to you!

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.

None yet

2 participants