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

Embedded zsh code sometimes not highlighted #21

Closed
lacygoill opened this issue Feb 19, 2019 · 19 comments
Closed

Embedded zsh code sometimes not highlighted #21

lacygoill opened this issue Feb 19, 2019 · 19 comments

Comments

@lacygoill
Copy link

lacygoill commented Feb 19, 2019

Hello,

According to the readme page of Vim's default markdown syntax plugin (and :h rmd.vim), one can embed some code in a markdown file, provided that it's surrounded in a fenced code block, and the variable g:markdown_fenced_languages is set correctly.

For example, if you set the variable like this:

let g:markdown_fenced_languages = ['vim']

And you write this in a markdown file:

```vim
echo 'hello'
```

echo 'hello' should be highlighted like it would be in a Vimscript file.


Some syntax groups defined in $VIMRUNTIME/syntax/zsh.vim use the argument contains=TOP.
I think that because of this, sometimes, when a zsh code is embedded in a markdown file, some part of it may not be highlighted.

Here's a MWE:

$ cat <<'EOF' >/tmp/md.md
```zsh
alias_is_it_free() {
  emulate -L zsh

  # make sure the name hasn't been taken yet
  printf -- 'type %s:\n' "$1"
  type "$1"

  # make sure the name won't shadow a future command
  printf -- '\napt-file -x search "/%s$":\n' "$1"
  apt-file -x search "/$1$"
}
```
EOF

The previous shell command creates the file /tmp/md.md, in which there's a fenced code block containing a zsh function. The next one starts Vim with a minimum of configuration, and opens /tmp/md.md:

$ vim -Nu NONE --cmd 'syntax on | let g:markdown_fenced_languages = ["zsh"] | set noloadplugins' /tmp/md.md

The embedded zsh code is not highlighted:

gif

For the time being, I deal with this issue by redefining the syntax groups zshSubst, zshBrackets, zshOldSubst, and replacing contains=TOP with contains=@markdownHighlightzsh, but only when the cluster @markdownHighlightzsh exists.

Here's how it works:

$ cat <<'EOF' >/tmp/vimrc
fu! s:fix_embedding() abort
    if execute('syn list @markdownHighlightzsh', 'silent!') !~# 'markdownHighlightzsh'
        return
    endif
    syn clear zshSubst zshBrackets zshOldSubst

    syn region  zshSubst    matchgroup=zshSubstDelim
                            \ start=/\$(/ skip=/\\)/ end=/)/
                            \ transparent
                            \ fold
                            \ contains=@markdownHighlightzsh
                            \ contained
    syn region  zshSubst    matchgroup=zshSubstDelim
                            \ start=/\${/ skip=/\\}/ end=/}/
                            \ fold
                            \ contains=@zshSubst,zshBrackets,zshQuoted,zshString
                            \ contained

    syn region  zshBrackets start=/{/ skip=/\\}/ end=/}/
                            \ transparent
                            \ fold
                            \ contained
    syn region  zshBrackets start='{' skip='\\}' end='}'
                            \ transparent
                            \ fold
                            \ contains=@markdownHighlightzsh
                            \ contained

    syn region  zshOldSubst matchgroup=zshSubstDelim
                            \ start=/`/ skip=/\\`/ end=/`/
                            \ fold
                            \ contains=@markdownHighlightzsh,zshOldSubst
                            \ contained
endfu

augroup syntax_fix_embedding
    au!
    au Syntax markdown call s:fix_embedding()
augroup END
EOF

The previous shell command creates a minimal vimrc whose only purpose is to redefine the 3 syntax groups mentioned earlier. The next one starts Vim with this minimal vimrc:

$ vim -Nu /tmp/vimrc --cmd 'syntax on | let g:markdown_fenced_languages = ["zsh"] | set noloadplugins' /tmp/md.md

This time, the zsh code is correctly highlighted:
gif

Unfortunately, I think this is brittle, as it could probably break if the code of the zsh syntax plugin changed in the future.

I wondered whether it would be possible for the plugin to use contains=TOP only on the condition that the cluster @markdownHighlightzsh does not exist. If it does, it would use contains=@markdownHighlightzsh instead.

But there's another issue. The default markdown syntax plugin uses the cluster @markdownHighlightzsh, but I use my own syntax plugin which uses the cluster @markdownEmbedzsh. And maybe other markdown syntax plugins use yet another name for this kind of cluster.

Do you think it would be possible to add an option that the user could set with the name of their cluster? By default, it would be set with markdownHighlightzsh, because that's what the default markdown syntax plugin uses; but the user could override it in their vimrc.

If you think an option is too much, I can understand. I could replace @markdownEmbedzsh with @markdownHighlightzsh in my plugin.


For reference, here are some links to similar issues:

derekwyatt/vim-scala#59
vim-pandoc/vim-pandoc-syntax#54


Feel free to close the issue if you think it's not important.
Thank you very much for your plugin.

@lacygoill
Copy link
Author

lacygoill commented Feb 19, 2019

I've just noticed that you need to add the contained argument in the definition of zshOldSubst, otherwise you can't conceal the fences around the code with something like this in your markdown syntax plugin:

syn region markdownFencedCodeBlock
    \ matchgroup=markdownCodeDelimiter
    \ start=/^```\S*\s*$/
    \ end=/^```\ze\s*$/
    \ keepend
    \ concealends

I'm not sure whether it means contained should be added to the 3 syntax groups when the zsh syntax plugin is sourced to embed some code.

@lacygoill
Copy link
Author

lacygoill commented Feb 19, 2019

Actually, if zshOldSubst is defined without contained, the stack of syntax items is wrong. For example, if you position the cursor on printf in the previous markdown file, the output of :echo reverse(map(synstack(line('.'), col('.')), {i,v -> synIDattr(v, 'name')})) is:

['zshCommands', 'zshBrackets', 'zshBrackets', 'zshOldSubst']

While it should be:

['zshCommands', 'zshBrackets', 'zshBrackets', 'markdownHighlightzsh']

Besides, if you try to add another fenced code block for a vimscript code, it's wrongly highlighted as a zsh code instead. contained seems to fix the issue.

@lacygoill
Copy link
Author

lacygoill commented Feb 19, 2019

Sorry, I was wrong: you wouldn't need to add contained at all, because when the markdown syntax plugin runs :syn include, Vim automatically adds contained to all the zsh syntax items.
However, I currently need to add contained, because I redefine the items with a :syn clear ... and a :syn region ....

@lacygoill
Copy link
Author

After having thought about this a little more, I think the issue is not in the zsh plugin, because you could try to include zsh code in any arbitrary number of clusters, and not necessarily in a markdown file. It's probably up to the user to find a workaround.

@chrisbra
Copy link
Owner

I think it is a valid request. I'll have a look to do something similar to the vim-scala commit. That should make it simpler for embedding zsh into other syntax languages.

@chrisbra chrisbra reopened this Feb 22, 2019
@danielshahaf
Copy link
Contributor

I've ran into the contains=TOP issue while trying write a syntax file for a custom language that embeds zsh.vim and I thought it was a Vim bug; see: https://bugs.debian.org/947120:

According to syntax.c:in_id_list(), "TOP" is indeed supposed to only accept groups that are defined in the same [syntax file].

@chrisbra
Copy link
Owner

Yes, I believe this is a Vim particularity (or bug). I remember having checked the source several years ago and noticing that behaviour, however back then I did not easily see a fix for this.

@chrisbra
Copy link
Owner

@danielshahaf @lacygoill I think I have worked around the issue now. Please check

@lacygoill
Copy link
Author

lacygoill commented Jan 20, 2020

Yes, I've just tested the new syntax plugin, and it fixes the issue; a fenced zsh codeblock embedded inside a markdown file is correctly highlighted as zsh code. Thank you very much for the fix.

@danielshahaf
Copy link
Contributor

danielshahaf commented Jan 20, 2020

@chrisbra Thanks for the patch! However, in my case the fix doesn't quite work. Let me describe the symptoms:

  1. With e29a10b of vim-zsh and current master (e626f57613ad2dacd7fe7244f6a33de5c8f03f9e) of Util/ztst-syntax.vim, if I view a random test case in A01grammar.ztst with filetype=ztst, then it gets highlighted correctly: the text Unbalanced parentheses and spaces with zsh pattern is highlighted with the Title highlight group. It remains highlighted correctly if I call append(810, "\x20() {}") (the leading space is significant to ztst syntax; the escape sequence is just to prevent GitHub from eating it whilst rendering this comment).

  2. If I upgrade to the next commit of vim-zsh, a0f17e0, or to current master (25c49bd), then the once I append () {} [still with a leading space], the highlighting is thrown off; the aforementioned text is no longer highlighted as Title. I found that if I then append another right brace (to form () {} }) the highlighting comes back in sync.

I've played a bit with new :execute line and found that if I change contains='.s:contained.' to contains=@nosuchgroup then the () {} case is highlighted correctly too. (That's obviously not the fix; it's only a hint as to what's going on.)

I don't know what causes this. It could very well be that the zsh.vim change has unearthed a latent ztst.vim bug. In that case, hints would be appreciated. :-)

@danielshahaf
Copy link
Contributor

The following patch seems to fix it:

--- a/syntax/zsh.vim
+++ b/syntax/zsh.vim
@@ -371,9 +371,9 @@ syn region  zshMathSubst        matchgroup=zshSubstDelim transparent
                                 \ start='\$((' skip='\\)' end='))'
                                 \ contains=zshParentheses,@zshSubst,zshNumber,
                                 \ @zshDerefs,zshString keepend fold
-syn region  zshBrackets         contained transparent start='{' skip='\\}'
+syn region  zshBrackets         contained transparent start='{'ms=s+1 skip='\\}'
                                 \ end='}' fold
-exe 'syn region  zshBrackets    transparent start=/{/ skip=/\\}/ end=/}/ contains='.s:contained. ' fold'
+exe 'syn region  zshBrackets    transparent start=/{/ms=s+1 skip=/\\}/ end=/}/ contains='.s:contained. ' fold'
 syn region  zshSubst            matchgroup=zshSubstDelim start='\${' skip='\\}'
                                 \ end='}' contains=@zshSubst,zshBrackets,zshQuoted,zshString fold
 exe 'syn region  zshOldSubst    matchgroup=zshSubstDelim start=/`/ skip=/\\[\\`]/ end=/`/ contains='.s:contained. ',zshOldSubst fold'

I tried this because I suspected that both the l374 and l376 definitions matched the opening brace, so I hoped two closing braces would terminate the region and then (at the end of the line, in my case) relinquish control back to the enclosing syntax file.

@chrisbra
Copy link
Owner

I suppose the zsh syntax file does not correctly determine that it is being embedded. Perhaps I need to detect another cluster as well.

@danielshahaf can you please share the test case? I'd like to have a look. In particular, I am wondering how the :set filetype=ztst has to do with it. Where is that filetype defined?

or other way around: What do I need to do to have such a test case opened in Vim with the filetype/syntax scripts set to zsh. Thanks.

@danielshahaf
Copy link
Contributor

@chrisbra Sure. ztst.vim is defined here: https://github.com/zsh-users/zsh/blob/master/Util/ztst-syntax.vim. Sorry, I forgot to make it a link in my previous comment. If you install that file as ~/.vim/syntax/ztst.vim, open https://github.com/zsh-users/zsh/blob/master/Test/A01grammar.ztst, and manually :set filetype=ztst, then lines that have a space in the first column should be highlighted as zsh. Then, adding an () {} to the zsh code will throw off the highlighting of non-zsh-code parts of the file (lines that start with a non-whitespace).

ztst.vim includes zsh.vim under the @zsh cluster, like pandoc.

Does this answer your question? If not, I'd be happy to provide any info you need.

@chrisbra
Copy link
Owner

chrisbra commented Jan 21, 2020

that is strange. Not sure what is causing it. However it seems like :syn sync fromstart fixes the problem. It can either be set by your ztst syntax script (or I can increase the maxline setting from syntax/zsh.vim, I have found that setting :syn sync minlines=50 maxlines=150 within zsh syntax fixes it also [but the syn sync from ztst syntax overrules is])

@danielshahaf
Copy link
Contributor

However it seems like :syn sync fromstart fixes the problem.

I issued this command manually and it did not fix the problem for me. I also tried deleting lines 1 through 800, so the aforelinked test case (nominally on lines 801-814) would be the very first thing in the file, and could still see the wrong highlighting then. I'm not sure why I'm seeing different behaviour than you do. I'm attaching my :version output below, in case it's relevant.

% dpkg -l vim-gtk | sed -ne '$p'
ii  vim-gtk        2:8.1.0875-5 amd64        Vi IMproved - enhanced vi editor - with GTK2 GUI
% vim --version
VIM - Vi IMproved 8.1 (2018 May 18, compiled Jun 15 2019 16:41:15)
Included patches: 1-875, 878, 884, 948, 1046, 1365-1368, 1382, 1401
Modified by team+vim@tracker.debian.org
Compiled by team+vim@tracker.debian.org
Huge version with GTK2 GUI.  Features included (+) or not (-):
+acl               +extra_search      +mouse_netterm     +tag_old_static
+arabic            +farsi             +mouse_sgr         -tag_any_white
+autocmd           +file_in_path      -mouse_sysmouse    +tcl
+autochdir         +find_in_path      +mouse_urxvt       +termguicolors
-autoservername    +float             +mouse_xterm       +terminal
+balloon_eval      +folding           +multi_byte        +terminfo
+balloon_eval_term -footer            +multi_lang        +termresponse
+browse            +fork()            -mzscheme          +textobjects
++builtin_terms    +gettext           +netbeans_intg     +textprop
+byte_offset       -hangul_input      +num64             +timers
+channel           +iconv             +packages          +title
+cindent           +insert_expand     +path_extra        +toolbar
+clientserver      +job               +perl              +user_commands
+clipboard         +jumplist          +persistent_undo   +vartabs
+cmdline_compl     +keymap            +postscript        +vertsplit
+cmdline_hist      +lambda            +printer           +virtualedit
+cmdline_info      +langmap           +profile           +visual
+comments          +libcall           -python            +visualextra
+conceal           +linebreak         +python3           +viminfo
+cryptv            +lispindent        +quickfix          +vreplace
+cscope            +listcmds          +reltime           +wildignore
+cursorbind        +localmap          +rightleft         +wildmenu
+cursorshape       +lua               +ruby              +windows
+dialog_con_gui    +menu              +scrollbind        +writebackup
+diff              +mksession         +signs             +X11
+digraphs          +modify_fname      +smartindent       -xfontset
+dnd               +mouse             +startuptime       +xim
-ebcdic            +mouseshape        +statusline        +xpm
+emacs_tags        +mouse_dec         -sun_workshop      +xsmp_interact
+eval              +mouse_gpm         +syntax            +xterm_clipboard
+ex_extra          -mouse_jsbterm     +tag_binary        -xterm_save
   system vimrc file: "$VIM/vimrc"
     user vimrc file: "$HOME/.vimrc"
 2nd user vimrc file: "~/.vim/vimrc"
      user exrc file: "$HOME/.exrc"
  system gvimrc file: "$VIM/gvimrc"
    user gvimrc file: "$HOME/.gvimrc"
2nd user gvimrc file: "~/.vim/gvimrc"
       defaults file: "$VIMRUNTIME/defaults.vim"
    system menu file: "$VIMRUNTIME/menu.vim"
  fall-back for $VIM: "/usr/share/vim"
Compilation: gcc -c -I. -Iproto -DHAVE_CONFIG_H -DFEAT_GUI_GTK  -pthread -I/usr/include/gtk-2.0 -I/usr/lib/x86_64-linux-gnu/gtk-2.0/include -I/usr/include/gio-unix-2.0 -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pixman-1 -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/pango-1.0 -I/usr/include/harfbuzz -I/usr/include/pango-1.0 -I/usr/include/fribidi -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/uuid -I/usr/include/freetype2 -I/usr/include/libpng16 -Wdate-time  -g -O2 -fdebug-prefix-map=/build/vim-4Pursk/vim-8.1.0875=. -fstack-protector-strong -Wformat -Werror=format-security -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1       
Linking: gcc   -L. -Wl,-z,relro -Wl,-z,now -fstack-protector -rdynamic -Wl,-export-dynamic -Wl,-E  -Wl,-z,relro -Wl,-z,now -Wl,--as-needed -o vim   -lgtk-x11-2.0 -lgdk-x11-2.0 -lpangocairo-1.0 -latk-1.0 -lcairo -lgdk_pixbuf-2.0 -lgio-2.0 -lpangoft2-1.0 -lpango-1.0 -lgobject-2.0 -lglib-2.0 -lfontconfig -lfreetype -lSM -lICE -lXpm -lXt -lX11 -lXdmcp -lSM -lICE  -lm -ltinfo -lnsl  -lselinux  -lacl -lattr -lgpm -ldl  -L/usr/lib -llua5.2 -Wl,-E  -fstack-protector-strong -L/usr/local/lib  -L/usr/lib/x86_64-linux-gnu/perl/5.28/CORE -lperl -ldl -lm -lpthread -lcrypt  -L/usr/lib/python3.7/config-3.7m-x86_64-linux-gnu -lpython3.7m -lcrypt -lpthread -ldl -lutil -lm -L/usr/lib/x86_64-linux-gnu -ltcl8.6 -ldl -lz -lpthread -lm -lruby-2.5 -lpthread -lgmp -ldl -lcrypt -lm     
% 

@danielshahaf
Copy link
Contributor

As to maxlines: in ztst.vim I set maxlines=1 to work around other issues. For example, in D02glob.ztst the # character on line 638 is not taken for a comment sign, which causes the apostrophe in the word don't to be taken as the start of a multiline single quoted string literal. Setting maxlines=1 makes it so as soon as the first line on the screen is line 640 or further down the file, the highlighting is back in sync. (The same happens, but in reverse, with (#b) in patterns, which is taken to be a comment starter even though it's not one.)

@chrisbra
Copy link
Owner

Hm, looks like I was wrong or I misunderstood something. Earlier you said:

I tried this because I suspected that both the l374 and l376 definitions matched the opening brace,

I actually always thought this is not possible, but I believe this is indeed what happened. With commit e29a10b zshBrackets was defined as ...contains=TOP, which meant, that all syntax groups that have the contained keyword are not allowed (so the first zshBracket syntax group was not allowed to match).

However with commit a0f17e0 zshBrackets was defined as contains=@zsh, which seems to include to contain zshBrackets again. And that seems to be the case here.

So let me include your suggested change and see if somebody complains. If yes, we might have to backout a0f17e0 again, as it seems to cause more harm that it solves.

chrisbra added a commit that referenced this issue Jan 23, 2020
When zsh is embedded into other syntaxes, it can happen, that the
zshBrackets (and possibly other groups as well) match several times on
the starting a syntax region.

See e.g. #21 (comment)

So include a match-offset of one character so that the same group cannot
match at the same time several times.

Signed-off-by: Christian Brabandt <cb@256bit.org>
Signed-off-by: Daniel Shahaf <danielsh@apache.org>
@chrisbra
Copy link
Owner

regarding your other message, I think I see why the comments are note matched. I believe the issue is this:

https://github.com/zsh-users/zsh/blob/e626f57613ad2dacd7fe7244f6a33de5c8f03f9e/Util/ztst-syntax.vim#L28

 syn match  ztstPayload             /^\s\+\zs.*/ contains=@zsh

I believe this match makes it impossible to have the zshComment match at the same time, because both patterns like to anchor against the start of the line:

For reference the zshComment

This requires either a space or the start of line before the comment. However, since the ztstPayload consumes anything until the # the comment cannot match. Now I could change the zshComment pattern to be more like start="#", however than it would probably match glob patterns qualifier, which shouldn't).

You can try, if this patch to the ztst syntax fixes it:

diff --git a/ztst-syntax.vim b/home/chrisbra/.vim/syntax/ztst.vim
index 01e4dae..8efdc71 100644
--- a/ztst-syntax.vim
+++ b/home/chrisbra/.vim/syntax/ztst.vim
@@ -52,7 +52,7 @@ syn region ztstFrequentExplanation start=// end=/$/ contained

 syn match  ztstDirective           /^%.*/

-syn match  ztstComment             /^#.*/
+syn match  ztstComment             /^\s*#.*/

 " Highlight those variables which are /de jure/ or /de facto/ APIs of the test
 " harness to the test files.

on the other side, I am not sure why (#b) would match a comment. It shouldn't according to my understanding of the patterns.

@danielshahaf
Copy link
Contributor

So let me include your suggested change and see if somebody complains. If yes, we might have to backout a0f17e0 again, as it seems to cause more harm that it solves.

Thanks! Yes, if that change causes a regression, we can always go back to the drawing board here.

You can try, if this patch to the ztst syntax fixes it:

Thank you very much for the diagnosis :-). Instead of letting ztstComment gobble the spaces, I'' remove the \zs from ztstPayload. It works too, and the comment to explain it was easier to write. Both changes DTRT on comments that are escaped by being made part of a multiline string, as in:

diff --git a/Test/D02glob.ztst b/Test/D02glob.ztst
index 3d7df94c9..a6ff087a9 100644
--- a/Test/D02glob.ztst
+++ b/Test/D02glob.ztst
@@ -635,6 +635,9 @@
   [[ a = ['!a'] ]]
 0:! not active in character class if quoted
 
+  echo 'foo
+    # bar
+  '
   # Actually, we don't need the quoting here,
   # c.f. the next test.  This just makes it look
   # more standard.

Cheers!

chrisbra added a commit that referenced this issue Jan 31, 2020
certain syntax elements are defined as `contained=TOP`. However, when
embedding the zsh syntax into another language like pandoc or markdown
does, this will prevent to match those syntax items, because the top level
items will then be used for the top level syntax language
markdown/pandoc and not zsh.

Therefore, check if the @zsh or @markdownHighlightzsh cluster is defined
and if so, instead of defining `contains=TOP` use the correct
`contains=@<cluster>` (which should be the cluster defined for the
embedded lanuage, e.g. either @zsh for pandoc or @markdownHighlightzsh
cluster for markdown).

closes #21
chrisbra added a commit that referenced this issue Jan 31, 2020
When zsh is embedded into other syntaxes, it can happen, that the
zshBrackets (and possibly other groups as well) match several times on
the starting a syntax region.

See e.g. #21 (comment)

So include a match-offset of one character so that the same group cannot
match at the same time several times.

Signed-off-by: Christian Brabandt <cb@256bit.org>
Signed-off-by: Daniel Shahaf <danielsh@apache.org>
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

No branches or pull requests

3 participants