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

GNU sed syntax coloring for comments #5876

Closed
Aster89 opened this issue Apr 2, 2020 · 11 comments
Closed

GNU sed syntax coloring for comments #5876

Aster89 opened this issue Apr 2, 2020 · 11 comments

Comments

@Aster89
Copy link
Contributor

Aster89 commented Apr 2, 2020

I think only one line should be changed in the file /usr/share/vim/vim82/syntax/sed.vim, from this

syn match sedComment	"^\s*#.*$"

to maybe this:

syn match sedComment	"\s*#.*$"

(I would have expected this to be too much, as by itself it would match even starting from a # which is a literal part of a regex or of a substitiution, or whatever, but this seems to be not the case, maybe because some other rule rules this out.)

The systax coloring for sed files should color the comments correctly for GNU sed, which allows leading comments on any line. Since this definition of the comment is less restrictive than then one for some prehistoric sed, this should not affect the syntax coloring of those scripts which are written for the latter.

The alternative is to keep having a feast of colors whenever I want to put a comment on the right of a line.

I can make a pull request myself, but it would a 1 char change commit.

@k-takata
Copy link
Member

k-takata commented Apr 3, 2020

Have you contacted the maintainer of the file?

@Aster89
Copy link
Contributor Author

Aster89 commented Apr 3, 2020

Based on the first lines of the file

" Vim syntax file
" Language:	sed
" Maintainer:	Haakon Riiser <hakonrk@fys.uio.no>
" URL:		http://folk.uio.no/hakonrk/vim/syntax/sed.vim
" Last Change:	2010 May 29

I have contacted the maintainer using that e-mail, but I received back 550 <hakonrk@ifi.uio.no>: Gone away, no forwarding address known.

@Aster89
Copy link
Contributor Author

Aster89 commented Jul 18, 2020

I've asked for help on StackOverflow. Nobody has answered yet there.

@dkearns
Copy link
Contributor

dkearns commented Sep 20, 2021

Can you give this a try? I haven't really tested it so it might require more work.

There was a problem with the substitute command flags syntax group matching anything and, therefore, a tail comment.

diff --git a/runtime/syntax/sed.vim b/runtime/syntax/sed.vim
index 63b39db81..4badb5560 100644
--- a/runtime/syntax/sed.vim
+++ b/runtime/syntax/sed.vim
@@ -17,7 +17,7 @@ syn match sedAddress  "[[:digit:]$]"
 syn match sedAddress   "\d\+\~\d\+"
 syn region sedAddress   matchgroup=Special start="[{,;]\s*/\(\\/\)\="lc=1 skip="[^\\]\(\\\\\)*\\/" end="/I\=" contains=sedTab,sedRegexpMeta
 syn region sedAddress   matchgroup=Special start="^\s*/\(\\/\)\=" skip="[^\\]\(\\\\\)*\\/" end="/I\=" contains=sedTab,sedRegexpMeta
-syn match sedComment   "^\s*#.*$"
+syn match sedComment   "#.*$"
 syn match sedFunction  "[dDgGhHlnNpPqQx=]\s*\($\|;\)" contains=sedSemicolon,sedWhitespace
 syn match sedLabel     ":[^;]*"
 syn match sedLineCont  "^\(\\\\\)*\\$" contained
@@ -34,8 +34,11 @@ syn region sedBranch matchgroup=sedFunction start="[bt]" matchgroup=sedSemicolon
 syn region sedRW       matchgroup=sedFunction start="[rw]" matchgroup=sedSemicolon end=";\|$" contains=sedWhitespace

 " Substitution/transform with various delimiters
-syn region sedFlagwrite            matchgroup=sedFlag start="w" matchgroup=sedSemicolon end=";\|$" contains=sedWhitespace contained
-syn match sedFlag          "[[:digit:]gpI]*w\=" contains=sedFlagwrite contained
+syn region  sedFlagWrite    matchgroup=sedFlag start="w" matchgroup=sedSemicolon end="$" contains=sedWhitespace contained
+syn region  sedFlagExecute  matchgroup=sedFlag start="e" matchgroup=sedSemicolon end="$" contains=sedWhitespace contained
+syn match   sedFlag        "[[:digit:]gIimMp]\+" nextgroup=sedFlagExecute,sedFlagWrite contained
+syn cluster sedFlags       contains=sedFlag.*
+
 syn match sedRegexpMeta            "[.*^$]" contained
 syn match sedRegexpMeta            "\\." contains=sedTab contained
 syn match sedRegexpMeta            "\[.\{-}\]" contains=sedTab contained
@@ -58,13 +61,13 @@ while __sed_i <= __sed_last
        if __sed_i != __at
            exe 'syn region sedAddress matchgroup=Special start=@\\'.__sed_delimiter.'\(\\'.__sed_delimiter.'\)\=@ skip=@[^\\]\(\\\\\)*\\'.__sed_delimiter.'@ end=@'.__sed_delimiter.'I\=@ contains=sedTab'
            exe 'syn region sedRegexp'.__sed_i  'matchgroup=Special start=@'.__sed_delimiter.'\(\\\\\|\\'.__sed_delimiter.'\)*@ skip=@[^\\'.__sed_delimiter.']\(\\\\\)*\\'.__sed_delimiter.'@ end=@'.__sed_delimiter.'@me=e-1 contains=sedTab,sedRegexpMeta keepend contained nextgroup=sedReplacement'.__sed_i
-           exe 'syn region sedReplacement'.__sed_i 'matchgroup=Special start=@'.__sed_delimiter.'\(\\\\\|\\'.__sed_delimiter.'\)*@ skip=@[^\\'.__sed_delimiter.']\(\\\\\)*\\'.__sed_delimiter.'@ end=@'.__sed_delimiter.'@ contains=sedTab,sedReplaceMeta keepend contained nextgroup=sedFlag'
+           exe 'syn region sedReplacement'.__sed_i 'matchgroup=Special start=@'.__sed_delimiter.'\(\\\\\|\\'.__sed_delimiter.'\)*@ skip=@[^\\'.__sed_delimiter.']\(\\\\\)*\\'.__sed_delimiter.'@ end=@'.__sed_delimiter.'@ contains=sedTab,sedReplaceMeta keepend contained nextgroup=@sedFlags'
        endif
     let __sed_i = __sed_i + 1
 endwhile
 syn region sedAddress matchgroup=Special start=+\\@\(\\@\)\=+ skip=+[^\\]\(\\\\\)*\\@+ end=+@I\=+ contains=sedTab,sedRegexpMeta
 syn region sedRegexp64 matchgroup=Special start=+@\(\\\\\|\\@\)*+ skip=+[^\\@]\(\\\\\)*\\@+ end=+@+me=e-1 contains=sedTab,sedRegexpMeta keepend contained nextgroup=sedReplacement64
-syn region sedReplacement64 matchgroup=Special start=+@\(\\\\\|\\@\)*+ skip=+[^\\@]\(\\\\\)*\\@+ end=+@+ contains=sedTab,sedReplaceMeta keepend contained nextgroup=sedFlag
+syn region sedReplacement64 matchgroup=Special start=+@\(\\\\\|\\@\)*+ skip=+[^\\@]\(\\\\\)*\\@+ end=+@+ contains=sedTab,sedReplaceMeta keepend contained nextgroup=@sedFlags

 " Since the syntax for the substituion command is very similar to the
 " syntax for the transform command, I use the same pattern matching
@@ -80,7 +83,8 @@ hi def link sedComment                Comment
 hi def link sedDelete          Function
 hi def link sedError           Error
 hi def link sedFlag            Type
-hi def link sedFlagwrite               Constant
+hi def link sedFlagWrite       Constant
+hi def link sedFlagExecute     Constant
 hi def link sedFunction                Function
 hi def link sedLabel           Label
 hi def link sedLineCont                Special

@Aster89
Copy link
Contributor Author

Aster89 commented Sep 21, 2021

I don't write Sed scripts everyday, so I don't have much to test. However, referring to this answer of mine on stackoverflow, I've picked up this Sed script

#!/usr/bin/env -S sed -Ef
s/(\[33m)([0-9a-f]{7,})(\[m)(.*)/\1\n\2\n\3\4/
h
s/^.*(\n[0-9a-f]{7,}\n.*|$)/\1/
x
s/\n[0-9a-f]{7,}\n.*$//
y/\\\//\/\\¯_/
G
s/\n([0-9a-f]{7,})\n/\1/
s/\n//

and added a trailing #ciao to each line. The result is that the single letter commands h, x, and G have a red background, which shouldn't be the case:

#!/usr/bin/env -S sed -Ef
s/(\[33m)([0-9a-f]{7,})(\[m)(.*)/\1\n\2\n\3\4/#caio
h#caio
s/^.*(\n[0-9a-f]{7,}\n.*|$)/\1/#caio
x#caio
s/\n[0-9a-f]{7,}\n.*$//#caio
y/\\\//\/\\¯_/#caio
G#caio
s/\n([0-9a-f]{7,})\n/\1/#caio
s/\n//#caio

Maybe this helps you?

@CervEdin
Copy link

CervEdin commented Jan 6, 2022

GNU sed doesn't seem to complain running echo | sed --posix 's#^#foo# #comment'

But FreeBSD sed returns sed: 1: "s#^#foo# #comment: bad flag in substitute command

I don't think trailing comments are posix compatible.

@Aster89 I would recommend using comments on the preceding line / or customizing your own syntax highlight rule

@chrisbra
Copy link
Member

chrisbra commented Jan 6, 2022

But FreeBSD sed returns sed: 1: "s#^#foo# #comment: bad flag in substitute command

Just curious, If you throw in a ; before the comment, does it than work (e.g. echo | sed --posix 's#^#foo#; #comment') ?

Anyhow, I just read up on https://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html and it doesn't mention, that the # is allowed to follow other commands, so this seems to be very unportable. (It looks like very old sed implementations, only allowed # at the very first line.

So let's close this issue then.

@chrisbra chrisbra closed this as completed Jan 6, 2022
@dkearns
Copy link
Contributor

dkearns commented Jan 9, 2022

I have a fix for this that I should dig up. I was side-tracked when I couldn't track down the maintainer.

Does this latest commentary suggest people would only want it supported behind a GNU extensions config variable?

I see that FreeBSD sed supports several GNU extensions already so perhaps opting into POSIX-only support would be better?

@CervEdin
Copy link

CervEdin commented Jan 9, 2022

Just curious, If you throw in a ; before the comment, does it than work (e.g. echo | sed --posix 's#^#foo#; #comment') ?

Yes. Both GNU and FreeBSD.
That would be equivalent of sed -e 's#^#foo#' -e ' #comment'
or

s#^#foo#
 #comment

Anyhow, I just read up on https://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html and it doesn't mention, that the # is allowed to follow other commands, so this seems to be very unportable. (It looks like very old sed implementations, only allowed # at the very first line.

Note that EOL comment doesn't work with stuff like

s#^#foo#w /dev/stderr ; #write to stderr

even in GNU.

I'd personally recommend not using EOL comments in sed.

Whether or not the syntax highlighting should warn or not, ¯_(ツ)_/¯

@chrisbra
Copy link
Member

Does this latest commentary suggest people would only want it supported behind a GNU extensions config variable?

@dkearns if you still want to contribute this, feel free to open a PR with those changes. Adding this behind a POSIX or GNU configuration variable seems to be acceptable, even if that is not always (fully) supported. Perhaps just mention this limitation at :h ft-sed-syntax, which already mentions a similar limitation for the y command.

@jacktose
Copy link

jacktose commented Oct 12, 2022

It doesn't look like this was ever changed, so I've opened #11353. @Aster89 & @dkearns, I'd love your input or improvements.

I see that FreeBSD sed supports several GNU extensions already so perhaps opting into POSIX-only support would be better?

I chose to make end-of-line comments allowed by default, since 1) it sounds like that's the majority case, and 2) I can't clearly tell from the docs for GNU sed, BSD sed, and BSD gsed what forms of comments are supported in which versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants