Skip to content

Bash: Added support for parameters and the java and sysctl commands. #3505

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.

Sign up for GitHub

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

Jump to bottom
Merged
RunDevelopment merged 18 commits into from
Jul 25, 2022
Merged

Bash: Added support for parameters and the java and sysctl commands. #3505

RunDevelopment merged 18 commits into from
Jul 25, 2022

Conversation

@wangliang181230
Copy link
Contributor

@wangliang181230 wangliang181230 commented Jul 25, 2022 •
edited
Loading

Added support for parameters:

  1. -key
  2. -key=value
  3. --key
  4. --key=value
  5. --aaa.key
  6. --aaa.key=value
  7. -XX:xxxxx
  8. -XX:xxxxx=value
  9. -XX:+xxxxx
  10. -XX:+xxxxx=value
  11. -XX:-xxxxx
  12. -XX:-xxxxx=value

Added more commands:

  1. java
  2. sysctl

Optimize assign-left:

  1. aaa.bbb=111

Display effect:

图片

@wangliang181230
prism-bash.js: add more functions
3625e86 
1. `|sysctl|`
2. the docker functions: `|docker|` -> `docker(\s+(run|start|stop|restart|kill|rm|pause|unpause|create|exec|ps|inspect|top|attach|events|logs|wait|export|port|commit|cp|diff|login|pull|push|search|images|rmi|tag|build|history|save|load|import|info|version))?|`
@github-actions
Copy link

github-actions bot commented Jul 25, 2022 •
edited
Loading

JS File Size Changes (gzipped)

A total of 1 files have changed, with a combined diff of +49 B (+1.7%).

Details
file master pull size diff % diff
components/prism-bash.min.js 2.89 KB 2.94 KB +49 B +1.7%

Generated by 🚫 dangerJS against b1570c8

RunDevelopment
RunDevelopment requested changes Jul 25, 2022
View reviewed changes
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.

Thank you for the PR @wangliang181230!

I left you a comment on the assignment token. The addition of sysctl is fine.

However, the docker command additions are incorrect. docker run is not a command. docker is a command and run is just a parameter (that docker just so happens to interpret as a sub command). So while the highlighting might be visually pleasing, I think it is incorrect and should be removed.

Also, please add update the tests. We have over a thousand test files and some of them changed. Please run npm run test:languages -- --update to update them all.

@wangliang181230
Copy link
Contributor Author

wangliang181230 commented Jul 25, 2022

However, the docker command additions are incorrect. docker run is not a command. docker is a command and run is just a parameter (that docker just so happens to interpret as a sub command). So while the highlighting might be visually pleasing, I think it is incorrect and should be removed.

Ok, i will remove the docker command.

Also, please add update the tests. We have over a thousand test files and some of them changed. Please run npm run test:languages -- --update to update them all.

Ok.

wangliang181230 and others added 2 commits July 25, 2022 16:39
@wangliang181230 @RunDevelopment
Update components/prism-bash.js
cc1c38c 
Suggested change.

Co-authored-by: Michael Schmidt <[email protected]>
@wangliang181230
Update prism-bash.js
c0dabc6 
remove docker commands.
@wangliang181230 wangliang181230 changed the title prism-bash.js: add more functions prism-bash.js: add function sysctl Jul 25, 2022
RunDevelopment
RunDevelopment requested changes Jul 25, 2022
View reviewed changes
@wangliang181230 wangliang181230 changed the title prism-bash.js: add function sysctl prism-bash.js: add function sysctl and java Jul 25, 2022
@wangliang181230 wangliang181230 changed the title prism-bash.js: add function sysctl and java prism-bash.js: add function sysctl and java, support parameter Jul 25, 2022
@wangliang181230 wangliang181230 changed the title prism-bash.js: add function sysctl and java, support parameter prism-bash.js: add functions sysctl and java, support parameter Jul 25, 2022
@wangliang181230 wangliang181230 changed the title prism-bash.js: add functions sysctl and java, support parameter prism-bash.js: add functions java and sysctl, support parameter Jul 25, 2022
RunDevelopment
RunDevelopment requested changes Jul 25, 2022
View reviewed changes
@RunDevelopment
Copy link
Member

RunDevelopment commented Jul 25, 2022

Also, please create a test case for the new parameter token. I appreciate the screenshots, but having this code as tests would be better. Please create the file tests/languages/bash/parameter_feature.test and paste your test code into it. After that, please run npm run test:languages -- --update.

RunDevelopment
RunDevelopment reviewed Jul 25, 2022
View reviewed changes
RunDevelopment
RunDevelopment reviewed Jul 25, 2022
View reviewed changes
@wangliang181230 @RunDevelopment
Update components/prism-bash.js
1997f43 
Suggested change.

Co-authored-by: Michael Schmidt <[email protected]>
@RunDevelopment
Copy link
Member

RunDevelopment commented Jul 25, 2022

Thank you @wangliang181230!

To make the CI pass, please run npm run build and commit components/prism-bash.min.js. Then I can merge this PR.

@wangliang181230 wangliang181230 changed the title prism-bash.js: add functions java and sysctl, support parameter prism-bash.js: support parameter, add functions java and sysctl. Jul 25, 2022
@RunDevelopment
Copy link
Member

RunDevelopment commented Jul 25, 2022

npm told you the problem:

npm WARN lifecycle The node binary used for scripts is C:\Program Files\nodejs\node.exe but npm is using D:\java\graalvm-ce-java8-20.3.0\jre\languages\js\bin\node.exe itself. Use the --scripts-prepend-node-path option to include the path for the node binary npm was executed with.

What's this strange node program you're using?

@wangliang181230
Copy link
Contributor Author

wangliang181230 commented Jul 25, 2022

Thank you @wangliang181230!

To make the CI pass, please run npm run build and commit components/prism-bash.min.js. Then I can merge this PR.

done.

@RunDevelopment
Copy link
Member

RunDevelopment commented Jul 25, 2022

That didn't work. You might have accidentally installed the wrong uglify version. Could you try running npm ci && npm run build?

@wangliang181230
Copy link
Contributor Author

wangliang181230 commented Jul 25, 2022 

PS E:\Workspace_Java\wangliang181230\prism> npm version
{                          
  prismjs: '1.28.0',       
  npm: '6.14.6',           
  ares: '1.16.0',          
  brotli: '1.0.7',         
  http_parser: '2.9.3',    
  llhttp: '2.1.2',         
  modules: '72',           
  napi: '6',               
  nghttp2: '1.41.0',       
  node: '12.18.4',
  openssl: '1.1.1g',       
  uv: '1.38.0',
  v8: '7.8.279.23-node.39',
  zlib: '1.2.11'
}

@wangliang181230
Copy link
Contributor Author

wangliang181230 commented Jul 25, 2022 •
edited
Loading

That didn't work. You might have accidentally installed the wrong uglify version. Could you try running npm ci && npm run build?

@RunDevelopment
I updated nodejs to the latest version 16.16.0 and run npm run build and succeeded.

@RunDevelopment RunDevelopment merged commit b9512b2 into PrismJS:master Jul 25, 2022
@RunDevelopment
Copy link
Member

RunDevelopment commented Jul 25, 2022

Thank you for contributing @wangliang181230!

@wangliang181230 wangliang181230 deleted the patch-1 branch July 25, 2022 13:26
@wangliang181230 wangliang181230 changed the title prism-bash.js: support parameter, add functions java and sysctl. Bash: Added support for parameters and the java and sysctl commands. Jul 25, 2022
@RunDevelopment RunDevelopment mentioned this pull request Jul 26, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RunDevelopment RunDevelopment RunDevelopment approved these changes

Assignees

No one assigned

Labels

enhancement language-definitions needs changes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@wangliang181230 @RunDevelopment