threads / discuss / 32581

about vim contrib support

Subject: about vim contrib support

## tl;dr

7 messages between Jan 10, 2013 and Jan 10, 2013.

replies: 6people: 3as markdown or json

Manlio Perillo· Jan 10, 2013, 11:17 UTC · lore
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi.

In the contrib/vim/README file there are instructions about how to setup git support with Vim builtin git syntax files.

However these instructions seems to be redundant, since the system filetype.vim file already have the autocmd rules.

The only issue I found is with:
autocmd BufNewFile,BufRead .msg.[0-9]*
	\ if getline(1) =~ '^From.*# This line is ignored.$' |
	\   setf gitsendemail |
	\ endif
It should be:
autocmd BufNewFile,BufRead [0-9]*.patch

IMHO it should contain some other checks, to make sure it is a patch generated by git format-patch, and not, as an example, a plain patch or a Mercurial patch.

By the way: I don't understand the purpose of gitsendemail syntax. On my system it does not highlight the diff.

I have implemented an alternate gitpatch syntax file, attached. What I would like to get, is to syntax highligth the commit subject message, but I'm not a Vim expert.

Regards   Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlDuo0sACgkQscQJ24LbaUTZMQCgm7QRylhxc5v4i4tHBfUXCl8o 36IAn3t72o/+5R/x1TF7r9mu85z6wY25 =b2l0 -----END PGP SIGNATURE-----

" Vim syntax file " Language: git format-patch message " Maintainer: Manlio Perillo " Filenames: [0-9]*.patch (first line is "From ... # This line is ignored.") " Last Change: 2014 Gen 10

if exists("b:current_syntax")
    finish
endif
syn case match

syn match gitsendemailComment "\%^From.*#.*" syn match gitsendemailComment "^GIT:.*"

if has("spell")
    syn spell toplevel
endif

syn include @gitcommitMessage syntax/gitcommit.vim syn region gitcommitMessage start=/^Subject: \@=/ end=/^$|^#\@=/ contains=@gitcommitMessage

hi def link gitsendemailComment Comment
let b:current_syntax = "gitpatch"
Jeff King· Jan 10, 2013, 11:39 UTC · re: Manlio Perillo · lore

Re: about vim contrib support

On Thu, Jan 10, 2013 at 12:17:31PM +0100, Manlio Perillo wrote:
Show 5 quoted lines
> In the contrib/vim/README file there are instructions about how to setup
> git support with Vim builtin git syntax files.
> 
> However these instructions seems to be redundant, since the system
> filetype.vim file already have the autocmd rules.

What version of vim do you have? As the README says, version 7.2 and on come with the files already, and you do not need to do anything. If you have an older version that does not ship with them, and you are pulling them down directly from the URLs provided, then your vim probably does not already have them in its stock filetype.vim.

Show 10 quoted lines
> The only issue I found is with:
> 
> autocmd BufNewFile,BufRead .msg.[0-9]*
> 	\ if getline(1) =~ '^From.*# This line is ignored.$' |
> 	\   setf gitsendemail |
> 	\ endif
> 
> It should be:
> 
> autocmd BufNewFile,BufRead [0-9]*.patch

It looks like .msg.[0-9] was originally used for send-email cover letters, and was changed to .gitsendemail.msg.* by commit eed6ca7. I think your [0-9]*.patch would match something else entirely (though it is still broken, of course, as .msg.* does not exist anymore).

I'd argue that we should just remove contrib/vim at this point. It has no actual files in it, only pointers to vim.org for pre-7.2 vim users. And that version was released in 2008, so the README is helping almost nobody at this point (if you are on an ancient platform, and are an avid enough vim user to download the syntax files, I suspect you would simply install a newer version of vim).

Show 6 quoted lines
> IMHO it should contain some other checks, to make sure it is a patch
> generated by git format-patch, and not, as an example, a plain patch or
> a Mercurial patch.
> 
> By the way: I don't understand the purpose of gitsendemail syntax.
> On my system it does not highlight the diff.

As far as I can tell, it is for cover letters, not for patches. Patches should already be handled by existing RFC822-message highlighting.

-Peff
Manlio Perillo· Jan 10, 2013, 11:51 UTC · re: Jeff King · lore

Re: about vim contrib support

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Il 10/01/2013 12:39, Jeff King ha scritto:
Show 10 quoted lines
> On Thu, Jan 10, 2013 at 12:17:31PM +0100, Manlio Perillo wrote:
> 
>> In the contrib/vim/README file there are instructions about how to setup
>> git support with Vim builtin git syntax files.
>>
>> However these instructions seems to be redundant, since the system
>> filetype.vim file already have the autocmd rules.
> 
> What version of vim do you have? As the README says, version 7.2 and on
> come with the files already, and you do not need to do anything.

Ah, right. I missed the first lines of the README file, sorry.

Show 14 quoted lines
> [...]
>> The only issue I found is with:
>>
>> autocmd BufNewFile,BufRead .msg.[0-9]*
>> 	\ if getline(1) =~ '^From.*# This line is ignored.$' |
>> 	\   setf gitsendemail |
>> 	\ endif
>>
>> It should be:
>>
>> autocmd BufNewFile,BufRead [0-9]*.patch
> 
> It looks like .msg.[0-9] was originally used for send-email cover
> letters,

Ok, thanks. I was assuming it was used for the generated patched.

Show 11 quoted lines
> and was changed to .gitsendemail.msg.* by commit eed6ca7. I
> think your [0-9]*.patch would match something else entirely (though it
> is still broken, of course, as .msg.* does not exist anymore).
> 
> [...]
>> By the way: I don't understand the purpose of gitsendemail syntax.
>> On my system it does not highlight the diff.
> 
> As far as I can tell, it is for cover letters, not for patches. Patches
> should already be handled by existing RFC822-message highlighting.
> 

.patch files are handled by diff highlight. What I would like to do is to use gitcommit syntax highlight, in order to also enable commit subject message hightlight.

Thanks   Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlDuqzYACgkQscQJ24LbaUQ5TgCfQPeX53KOsQDF6WJF1AaSpiRd NpMAn0GcffJwTA/etrnOnXAQctCKAY4W =IDVf -----END PGP SIGNATURE-----

Jeff King· Jan 10, 2013, 13:36 UTC · re: Manlio Perillo · lore

Re: about vim contrib support

On Thu, Jan 10, 2013 at 12:51:18PM +0100, Manlio Perillo wrote:
> .patch files are handled by diff highlight.
> What I would like to do is to use gitcommit syntax highlight, in order
> to also enable commit subject message hightlight.

Using the regular gitcommit highlighter would not make sense, as it is intended for the message templates seen when making a commit. Whereas format-patch .patch files have the headers as email headers.

You can load the mail header highlighting on top of diff highlighting like this (which only triggers for patches that look like emails):

  au FileType diff
    \ if getline(1) =~ '^From ' |
    \   unlet b:current_syntax |
    \   runtime! syntax/mail.vim |
    \ endif

But maybe there is something else that you wanted to highlight. It's not clear to me what you want from gitcommit's highlighting. Is it the "complain about long lines" highlighting? I think that you'd have to pull out of the gitcommit.vim and execute manually (and you'd have to tweak the regex to take into account the "Subject: [PATCH] bits).

-Peff
Jonathan Nieder· Jan 10, 2013, 20:54 UTC · re: Jeff King · lore

[PATCH] contrib/vim: simplify instructions for old vim support

Rely on the upstream filetype.vim instead of duplicating its rules in git's instructions for syntax highlighting support on pre-7.2 vim versions.

The result is a shorter contrib/vim/README. More importantly, it lets us punt on maintenance of the autocmd rules.

So now when we fix the upstream gitsendemail rule in light of commit eed6ca7, new git users stuck on old vim reading contrib/vim/README can automagically get the fix without any further changes needed to git.

Once the world has moved on to vim 7.2+ completely, we can get rid of these instructions, but for now if they are this simple it's effortless to keep them.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Jeff King wrote:
> I'd argue that we should just remove contrib/vim at this point. It has
> no actual files in it, only pointers to vim.org for pre-7.2 vim users.

I think that's reasonable. Of course we can still discuss enhancements to the vim support on this list, but ultimately it's easiest to distribute and document such work upstream in the usual way for vim plugins.

How about this patch?
 contrib/vim/README | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)
diff --git a/contrib/vim/README b/contrib/vim/README
index fca1e17..8f16d06 100644
--- a/contrib/vim/README
+++ b/contrib/vim/README
@@ -17,16 +17,6 @@ To install:
 
   1. Copy these files to vim's syntax directory $HOME/.vim/syntax
   2. To auto-detect the editing of various git-related filetypes:
-	$ cat >>$HOME/.vim/filetype.vim <<'EOF'
-	autocmd BufNewFile,BufRead *.git/COMMIT_EDITMSG    setf gitcommit
-	autocmd BufNewFile,BufRead *.git/config,.gitconfig setf gitconfig
-	autocmd BufNewFile,BufRead git-rebase-todo         setf gitrebase
-	autocmd BufNewFile,BufRead .msg.[0-9]*
-		\ if getline(1) =~ '^From.*# This line is ignored.$' |
-		\   setf gitsendemail |
-		\ endif
-	autocmd BufNewFile,BufRead *.git/**
-		\ if getline(1) =~ '^\x\{40\}\>\|^ref: ' |
-		\   setf git |
-		\ endif
-	EOF
+
+	$ curl http://ftp.vim.org/pub/vim/runtime/filetype.vim |
+		sed -ne '/^" Git$/, /^$/ p' >>$HOME/.vim/filetype.vim
-- 
1.8.1
Jeff King· Jan 10, 2013, 21:34 UTC · re: Jonathan Nieder · lore

Re: [PATCH] contrib/vim: simplify instructions for old vim support

On Thu, Jan 10, 2013 at 12:54:27PM -0800, Jonathan Nieder wrote:
Show 28 quoted lines
> Rely on the upstream filetype.vim instead of duplicating its rules in
> git's instructions for syntax highlighting support on pre-7.2 vim
> versions.
> 
> The result is a shorter contrib/vim/README.  More importantly, it lets
> us punt on maintenance of the autocmd rules.
> 
> So now when we fix the upstream gitsendemail rule in light of commit
> eed6ca7, new git users stuck on old vim reading contrib/vim/README can
> automagically get the fix without any further changes needed to git.
> 
> Once the world has moved on to vim 7.2+ completely, we can get rid of
> these instructions, but for now if they are this simple it's
> effortless to keep them.
> 
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> Jeff King wrote:
> 
> > I'd argue that we should just remove contrib/vim at this point. It has
> > no actual files in it, only pointers to vim.org for pre-7.2 vim users.
> 
> I think that's reasonable.  Of course we can still discuss enhancements
> to the vim support on this list, but ultimately it's easiest to
> distribute and document such work upstream in the usual way for vim
> plugins.
> 
> How about this patch?

Yeah, I think this makes sense. I'd be fine with removing it entirely, but it doesn't hurt to err on the conservative side and leave it there. It's not like it's generating a huge maintenance burden, and with your patch, there is even less to maintain.

Acked-by: Jeff King <peff@peff.net>
-Peff
Jonathan Nieder· Jan 10, 2013, 23:08 UTC · re: Jeff King · lore

Re: [PATCH] contrib/vim: simplify instructions for old vim support

Jeff King wrote:
> On Thu, Jan 10, 2013 at 12:54:27PM -0800, Jonathan Nieder wrote:
>> Rely on the upstream filetype.vim instead of duplicating its rules in
>> git's instructions for syntax highlighting support on pre-7.2 vim
>> versions.
[...]
Show 6 quoted lines
> Yeah, I think this makes sense. I'd be fine with removing it entirely,
> but it doesn't hurt to err on the conservative side and leave it there.
> It's not like it's generating a huge maintenance burden, and with your
> patch, there is even less to maintain.
>
> Acked-by: Jeff King <peff@peff.net>
Thanks for looking it over.

← back to recent threads