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

vim-command-loop.patch from Debian #74

Closed
blueyed opened this Issue Sep 17, 2014 · 13 comments

Comments

Projects
None yet
3 participants
@blueyed
Contributor

blueyed commented Sep 17, 2014

I've looked at Debian's patches to exuberant-ctags, and the following might be interesting for you:

From http://anonscm.debian.org/cgit/users/cjwatson/exuberant-ctags.git/tree/debian/patches/vim-command-loop.patch

From 15b3c591abd3b0e4334f9fba19a9b75330b33c26 Mon Sep 17 00:00:00 2001
From: Colin Watson <cjwatson@ubuntu.com>
Date: Sat, 15 Feb 2014 22:47:02 +0000
Subject: Fix infinite loop parsing vim commands

This happens if a non-alphanumeric character other than whitespace or '-' is
found before the first alphanumeric character after 'command'.

Bug: http://sourceforge.net/tracker/index.php?func=detail&aid=3214129&group_id=6556&atid=106556
Bug-Ubuntu: https://bugs.launchpad.net/bugs/736367
Forwarded: no
Last-Update: 2011-03-17

Patch-Name: vim-command-loop.patch

---
 vim.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/vim.c b/vim.c
index 4e6fba8..d17a1ba 100644
--- a/vim.c
+++ b/vim.c
@@ -405,7 +405,9 @@ static boolean parseCommand (const unsigned char *line)
      while (*cp && !isspace ((int) *cp))
        ++cp;
    }
-   } while ( *cp &&  !isalnum ((int) *cp) );
+       else
+           break;
+   } while ( *cp );

  if ( ! *cp )
  {

More patches are available at http://anonscm.debian.org/cgit/users/cjwatson/exuberant-ctags.git/tree/debian/patches/, but only http://anonscm.debian.org/cgit/users/cjwatson/exuberant-ctags.git/tree/debian/patches/python-disable-imports.patch seems to be missing, and I'm not sure if it's a good one.

@masatake masatake self-assigned this Sep 18, 2014

@masatake

This comment has been minimized.

Member

masatake commented Sep 19, 2014

Hi,

I tried the patch; and I got a question.

In your report at exuberant Bugs tracker, following example input is introduced:

command 'foo'

In this case what is exepcted as ctags output?
I got

'foo input.vim /&comand'foo'$/;" c

A single quote at the head of line is expected?

@blueyed

This comment has been minimized.

Contributor

blueyed commented Sep 19, 2014

The expected output would be "none" probably, because this isn't a valid command name (according to Colin, who fixed the bug): https://bugs.launchpad.net/ubuntu/+source/exuberant-ctags/+bug/736367/comments/1

@masatake

This comment has been minimized.

Member

masatake commented Sep 20, 2014

Do you mean "none" is that the output should be empty?

@masatake masatake added the Parsers label Sep 20, 2014

masatake added a commit that referenced this issue Sep 20, 2014

add tracking status of Debian
Inspired from report at #74
of Daniel Hahler.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
@blueyed

This comment has been minimized.

Contributor

blueyed commented Sep 20, 2014

Do you mean "none" is that the output should be empty?

I've meant that, but am not sure. Because the command name is invalid (E182), it should get ignored probably?!

Currently (using the patch) parses it as ':

'   Units/vim-command.d/input.vim   /^com ''$/;"    c

vim-lustyexplorer uses it in its Ruby code (https://github.com/sjbach/lusty/blob/master/autoload/lustyexplorer.vim#L161), which is used to call the command, but not to define it:

command "redraw"

The main issue here is maybe that the parser for vim should ignore any ruby, python etc blocks?!

@blueyed

This comment has been minimized.

Contributor

blueyed commented Sep 20, 2014

Fixed in ctags svn (differently): https://sourceforge.net/p/ctags/code/812/

@masatake

This comment has been minimized.

Member

masatake commented Sep 22, 2014

Thank you for notifying. I will use exuberent's code.

@masatake

This comment has been minimized.

Member

masatake commented Sep 22, 2014

It seems that the fix at exuberent conflicts with your fix for #72.
Debain's patch works well with your patch. I will debian's one.

masatake added a commit that referenced this issue Sep 22, 2014

resolve confliction between two patches for vim.c
After merging a fix(7fb36a2) for vim
parser, vim-command.d unit test case fails because the fix conflicts
with another fix (2fe9fd5).

So instead of 7fb36a2,
I use a patch from Debian
(http://anonscm.debian.org/cgit/users/cjwatson/exuberant-ctags.git/tree/debian/patches/vim-command-loop.patch)
which is suggested in #74.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
@vhda

This comment has been minimized.

Contributor

vhda commented Sep 22, 2014

Should we update the sourceforge git-svn branch and merge it?

@vhda

This comment has been minimized.

Contributor

vhda commented Sep 22, 2014

Ups, I just noticed that is what you did! :)

@blueyed

This comment has been minimized.

Contributor

blueyed commented Sep 22, 2014

How do the patches conflict?
Maybe only in the outcome of the unit test?

Without having verified it, these patches might behave different, in what gets parsed as a command.

I would prefer to use the upstream/Excuberant version of the patch.

@masatake

This comment has been minimized.

Member

masatake commented Sep 23, 2014

Yes, only in the outcome of the unit test(vim-command.d/input.vim).
How about this one http://pkgs.fedoraproject.org/cgit/ctags.git/tree/ctags-5.7-segment-fault.patch?

@blueyed

This comment has been minimized.

Contributor

blueyed commented Sep 23, 2014

👍

The patch from Fedora looks more sophisticated, especially because it ignores the invalid tags.

@masatake

This comment has been minimized.

Member

masatake commented Sep 25, 2014

Applied the patch at Fedora.
The tests are passed as expectedly.
Thank you for evaluation the patch.

@masatake masatake closed this Sep 25, 2014

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