Skip to content

syntax - sh.vim - Unify bashStatement, kshStatement and shStatement#18355

Closed
bfoersterling wants to merge 2 commits into
vim:masterfrom
bfoersterling:syntax-sh-unify-bashStatement-kshStatement-shStatement
Closed

syntax - sh.vim - Unify bashStatement, kshStatement and shStatement#18355
bfoersterling wants to merge 2 commits into
vim:masterfrom
bfoersterling:syntax-sh-unify-bashStatement-kshStatement-shStatement

Conversation

@bfoersterling

Copy link
Copy Markdown
Contributor

Changes

Following the discussion on the mailing list.

I have looked at all keywords and checked if they are built into bash,
dash, ksh, or sh, and if they are external commands.
The external programs are still in shStatement but they are visually
separated by a new vimscript statement in a new line.
Unfortunatelly there are a lot of differences between builtin commands
especially between the different Kornshell flavors and also Dash misses a lot
of builtins.
Maybe the external commands can also be moved to their own syntax group?
But the highlighting will probably stay the same so not sure if this is useful.

Restructured and simplified the logic of the kornshell flavors.
Tried to unify statements whenever possible.

Highlighting for the rpm command was removed as being too distribution specific.

Cleanup: formatted with gg=G and removed dead code.

Rerecorded Tests

  • testdir/dumps/sh_09_00.dump (let is not a keyword in dash)
  • testdir/dumps/sh_ksh2020_05.dump (rename is an ext. command that may be highlighted)
  • testdir/dumps/sh_ksh93u_05.dump (rename is an ext. command that may be highlighted)
  • testdir/dumps/sh_ksh93v_05.dump (rename is an ext. command that may be highlighted)
  • testdir/dumps/sh_ksh88_05.dump (rename is an ext. command that may be highlighted)
  • testdir/dumps/sh_sundrous_03.dump (xgrep should not be highlighted in bash)
$ let foo='bar'
dash: 1: let: not found

rename usually comes preinstalled through https://github.com/util-linux/util-linux/tree/master.

xgrep is neither a builtin in bash nor an external command that I could find.

- unified bashStatement, kshStatement and shStatement as much as
  possible
- separated builtin commands from external programs
- cleaned up kornshell flavor logic

Signed-off-by: bfoersterling <bjoern.foersterling@gmail.com>
Comment thread runtime/syntax/sh.vim Outdated
syn keyword bashSpecialVariables contained auto_resume BASH BASH_ALIASES BASH_ARGC BASH_ARGV BASH_CMDS BASH_COMMAND BASH_ENV BASH_EXECUTION_STRING BASH_LINENO BASHOPTS BASHPID BASH_REMATCH BASH_SOURCE BASH_SUBSHELL BASH_VERSINFO BASH_VERSION BASH_XTRACEFD CDPATH COLUMNS COMP_CWORD COMP_KEY COMP_LINE COMP_POINT COMPREPLY COMP_TYPE COMP_WORDBREAKS COMP_WORDS COPROC COPROC_PID DIRSTACK EMACS ENV EUID FCEDIT FIGNORE FUNCNAME FUNCNEST GLOBIGNORE GROUPS histchars HISTCMD HISTCONTROL HISTFILE HISTFILESIZE HISTIGNORE HISTSIZE HISTTIMEFORMAT HOME HOSTFILE HOSTNAME HOSTTYPE IFS IGNOREEOF INPUTRC LANG LC_ALL LC_COLLATE LC_CTYPE LC_MESSAGES LC_NUMERIC LINENO LINES MACHTYPE MAIL MAILCHECK MAILPATH MAPFILE OLDPWD OPTARG OPTERR OPTIND OSTYPE PATH PIPESTATUS POSIXLY_CORRECT PPID PROMPT_COMMAND PS0 PS1 PS2 PS3 PS4 PWD RANDOM READLINE_LINE READLINE_POINT REPLY SECONDS SHELL SHELLOPTS SHLVL TIMEFORMAT TIMEOUT TMPDIR UID
syn keyword bashStatement bind builtin caller compgen complete compopt declare dirs disown enable fg help history let logout mapfile popd pushd readarray shopt source suspend time
syn keyword bashStatement typeset nextgroup=shSetOption
syn keyword bashAdminStatement daemon reload restart start status stop

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

daemon is of little use, isn't it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Right, I have removed it in the latest commit.

reload, restart, start, status, and stop are not that useful either in my opinion, but maybe someone wants these for sys5init scripts.
And I don't think these script have to be in bash, so it should rather be named shAdminStatement and put in global scope.

@chrisbra

Copy link
Copy Markdown
Member

Thanks, I think this makes sense.

Comment thread runtime/syntax/sh.vim Outdated
" ===================
syn keyword shStatement break cd chdir continue eval exec exit kill newgrp pwd read readonly return shift test trap ulimit umask wait
" builtins:
syn keyword shStatement alias bg break cd continue command eval exec exit export fc getopts hash jobs local printf read readonly return shift times trap type ulimit umask unalias wait

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I haven't tried but this should break the shAlias match as the syn-keyword will have priority.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, it actually broke it for some reason.

I changed it back and added the test runtime/syntax/testdir/input/sh_bash_alias.sh.
Maybe you can check if it is correct now and if you have an edge case in mind add a test for it.

Thanks for the review.

@dkearns

dkearns commented Sep 22, 2025

Copy link
Copy Markdown
Contributor

xgrep is neither a builtin in bash nor an external command that I could find.

This is probably grep -X from AT&T's AST Toolkit. It gets a mention in the ksh man page.

- fixed alias syntax highlighting
- added test for bash alias syntax highlighting
- removed daemon keyword

Signed-off-by: bfoersterling <bjoern.foersterling@gmail.com>
@bfoersterling

Copy link
Copy Markdown
Contributor Author

xgrep is neither a builtin in bash nor an external command that I could find.

This is probably grep -X from AT&T's AST Toolkit. It gets a mention in the ksh man page.

I don't think this should be highlighted in Bash.

For the kornshell:
According to the tests it is available in ksh88, ksh93v, ksh93u and ksh2020.
I tested mksh from alpine's apk and ksh from Arch Linux' pacman (which seems to be ksh93) and there was no xgrep command.
But I guess it is better to keep it for the kornshells as I am not familiar with kornshell.

@chrisbra

Copy link
Copy Markdown
Member

thanks, let me include it

@chrisbra chrisbra closed this in 11bde1f Sep 23, 2025
zeertzjq added a commit to zeertzjq/neovim that referenced this pull request Sep 23, 2025
- unified bashStatement, kshStatement and shStatement as much as
  possible
- separated builtin commands from external programs
- cleaned up kornshell flavor logic
- fixed alias syntax highlighting
- added test for bash alias syntax highlighting
- removed daemon keyword

closes: vim/vim#18355

vim/vim@11bde1f

Co-authored-by: Bjoern Foersterling <bjoern.foersterling@gmail.com>
zeertzjq added a commit to neovim/neovim that referenced this pull request Sep 24, 2025
- unified bashStatement, kshStatement and shStatement as much as
  possible
- separated builtin commands from external programs
- cleaned up kornshell flavor logic
- fixed alias syntax highlighting
- added test for bash alias syntax highlighting
- removed daemon keyword

closes: vim/vim#18355

vim/vim@11bde1f

Co-authored-by: Bjoern Foersterling <bjoern.foersterling@gmail.com>
yilisharcs pushed a commit to yilisharcs/neovim that referenced this pull request Sep 24, 2025
…im#35895)

- unified bashStatement, kshStatement and shStatement as much as
  possible
- separated builtin commands from external programs
- cleaned up kornshell flavor logic
- fixed alias syntax highlighting
- added test for bash alias syntax highlighting
- removed daemon keyword

closes: vim/vim#18355

vim/vim@11bde1f

Co-authored-by: Bjoern Foersterling <bjoern.foersterling@gmail.com>
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.

3 participants