[m-rev.] for review: rewrite of vim syntax file

Sebastian Godelet sebastian.godelet at outlook.com
Tue Apr 14 17:26:09 AEST 2015

Hi Peter, thanks for the review.

I've trouble to apply the attached diff,
when I apply it to the patch I send on April 4th (or the one from master),
I get the following errors:
git apply --ignore-whitespace vim.diff
vim.diff:9: trailing whitespace.
   " pw - don't remember what this is about
vim.diff:10: trailing whitespace.
setlocal formatoptions=trcq
vim.diff:11: trailing whitespace.
setlocal indentexpr=cindent()
vim.diff:12: trailing whitespace.

vim.diff:21: trailing whitespace.
" pw - why would I want to shorten lines
error: patch failed: vim/ftplugin/mercury.vim:44
error: vim/ftplugin/mercury.vim: patch does not apply
error: patch failed: vim/syntax/mercury.vim:279
error: vim/syntax/mercury.vim: patch does not apply
Would you mind rebasing your diff to the current master (that one has 
the colouring for head clauses already addressed) if possible?
If that's not possible, then I'll do it by hand.

In your diff, you marked these <Fx> keyboard commands as conflicting:
    " <F6> attempts to wrap a call up with { } braces for DCG escapes.
-nnoremap <F6> I{ <ESC>%a }<ESC>j
+" pw - conflicts
+" nnoremap <F6> I{ <ESC>%a }<ESC>j

    " <F7> and <F8> comment and uncomment lines out respectively.
-nnoremap <F7> 0i% <ESC>j
-nnoremap <F8> :s/% //e<CR>j
+" pw - should only be applicable to mercury
+" nnoremap <F7> 0i% <ESC>j
+" nnoremap <F8> :s/% //e<CR>j

These I didn't touch (at least I can't recall that), as far as I 
remember there were like this in the original version.

Also about the comment line length, I just adjusted the line length to 
the one in the standard library, e.g. from io.m (77):
Before it was 80 chars but I remember that I once read on the mailing 
list that this was deemed to long once.
Maybe I just miscount. Hope you can clarify on this.

I'll address the other points later.

cheers Sebastian.

On 14/04/2015 12:52, Peter Wang wrote:
> On Sat, 4 Apr 2015 12:52:05 +0800, Sebastian Godelet <sebastian.godelet at outlook.com> wrote:
>> Hi,
>> after trying to improve some aspects of the existing Vim
>> syntax file provided in the Mercury distribution,
>> I came to the conclusion that it might be better to do a rewrite
>> which tries to be as backwards compatible as possible while still
>> allowing to add features to both the syntax file and the Vim
>> plugin script.
>> Do to the size of the diff I attached it rather than inlining it into
>> the email.
> Hi,
> I like that foreign code is no longer a sea of purple or cyan.
> Some of the changes are too much for me, so I customised it to look
> closer to what I had before.  I attached a diff in case any of it is
> useful for the official version.  (The colours are subjective but I like
> to reserve yellow for declarations and clause heads to stand out.)
> Other suggestions:
>    - make the variable matching optional, preferably off by default as I
>      find it distracting
>    - move more stuff into mercury_highlight_comment_special
>    - don't enable folding by default
>    - don't conceal by default.  The symbols are not standard Mercury and
>      may confuse.  They also look bad in some fonts.
> Peter

More information about the reviews mailing list