nits with new _git

nits with new _git

[Mikael Magnusson]

Now that pws fixed the crash, I'm updating to the new _git, so far
I've noticed these two things.

git log --pret<tab> goes into correction, same for git show.
% git log --pretty=
---- option
--pretty  -- pretty print commit messages
---- corrections (errors: 2)
--grep    -- limit commits to those with log messages matching the given pattern
--pretty  -- pretty print commit messages

Usually this would hint at some missing ret=0 somewhere, but I
couldn't find any. Unconditionally returning 0 from _git does 'fix' it
though, so it must be somewhere.

The other thing is that git branch -d -r <tab> doesn't complete remote branches.

-- 
Mikael Magnusson

Re: nits with new _git

[Mikael Magnusson]

Also noticed now, some options that take numbers just show '-M' as the
description instead of the actual string. For example git tag -n <tab>
and git fast-export --progress=<tab>, while git config
gui.blamehistoryctx <tab> properly shows 'number of days'.

On 14 May 2011 03:28, Mikael Magnusson <mikachu@gmail.com> wrote:
> Now that pws fixed the crash, I'm updating to the new _git, so far
> I've noticed these two things.
>
> git log --pret<tab> goes into correction, same for git show.
> % git log --pretty=
> ---- option
> --pretty  -- pretty print commit messages
> ---- corrections (errors: 2)
> --grep    -- limit commits to those with log messages matching the given pattern
> --pretty  -- pretty print commit messages
>
> Usually this would hint at some missing ret=0 somewhere, but I
> couldn't find any. Unconditionally returning 0 from _git does 'fix' it
> though, so it must be somewhere.
>
> The other thing is that git branch -d -r <tab> doesn't complete remote branches.
>
> --
> Mikael Magnusson
>



-- 
Mikael Magnusson

Re: nits with new _git

[Nikolai Weibull]

On Sat, May 14, 2011 at 03:46, Mikael Magnusson <mikachu@gmail.com> wrote:
> Also noticed now, some options that take numbers just show '-M' as the
> description instead of the actual string. For example git tag -n <tab>
> and git fast-export --progress=<tab>, while git config
> gui.blamehistoryctx <tab> properly shows 'number of days'.

I’ll look into these issues tomorrow.

Re: nits with new _git

[Nikolai Weibull]

On Sat, May 14, 2011 at 03:46, Mikael Magnusson <mikachu@gmail.com> wrote:
> Also noticed now, some options that take numbers just show '-M' as the
> description instead of the actual string. For example git tag -n <tab>
> and git fast-export --progress=<tab>, while git config
> gui.blamehistoryctx <tab> properly shows 'number of days'.

This is a problem with __git_guard_* not checking for compadd options.
 I’ll fix it tonight.

Is there any way that I could get commit access to

Completion/Unix/Command/_git

so that I can fix smaller issues like this one without first posting a
[PATCH] to zsh-workers?  I’m worried that the flood of smaller patches
drown out important ones and also may result in some going missed.

If so, my user name on SourceForge is pcppopper.

Thanks!

build problem with patch level 1.5301

[S. Cowles]

i have tried building patchlevel 1.5301 from cvs pulls (on various 
platforms) and get the error shown in the following excerpt of the build:


make[4]: Leaving directory `xx/origdist/zsh/zsh/Src/Zle'
Updated `zleparameter.mdh'.
echo 'timestamp for zleparameter.mdh against zleparameter.mdd' > zleparameter.mdhs
gawk -f ../../Src/makepro.awk zleparameter.c Src/Zle > zleparameter.syms
(echo '/* Generated automatically */'; sed -n '/^E/{s/^E//;p;}' < zleparameter.syms) \
                 > zleparameter.epro
(echo '/* Generated automatically */'; sed -n '/^L/{s/^L//;p;}' < zleparameter.syms) \
                 > `echo zleparameter.epro | sed 's/\.epro$/.pro/'`
make[4]: Entering directory `xx/origdist/zsh/zsh/Src/Zle'
make[4]: Leaving directory `xx/origdist/zsh/zsh/Src/Zle'
Updated `zleparameter.mdh'.
make[3]: Leaving directory `xx/origdist/zsh/zsh/Src/Zle'
make[2]: Leaving directory `xx/origdist/zsh/zsh/Src'
rm -f stamp-modobjs.tmp
make[2]: Entering directory `xx/origdist/zsh/zsh/Src'
gcc -c -I. -I../Src -I../Src -I../Src/Zle -I.  -DHAVE_CONFIG_H -Wall -Wmissing-prototypes -O2  -o builtin.o builtin.c
In file included from zsh.mdh:49:0,
                  from builtin.c:33:
mem.epro:5:35: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'last_heap_id'
mem.epro:15:48: error: expected ')' before 'heap_id'
make[2]: *** [builtin.o] Error 1
make[2]: Leaving directory `xx/origdist/zsh/zsh/Src'
make[1]: *** [modobjs] Error 2
make[1]: Leaving directory `xx/origdist/zsh/zsh/Src'
make: *** [all] Error 1

Re: build problem with patch level 1.5301

[Bart Schaefer]

On May 13,  8:13pm, S. Cowles wrote:
}
} gcc -c -I. -I../Src -I../Src -I../Src/Zle -I.  -DHAVE_CONFIG_H -Wall -Wmissing-prototypes -O2  -o builtin.o builtin.c
} In file included from zsh.mdh:49:0,
}                   from builtin.c:33:
} mem.epro:5:35: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'last_heap_id'

Hmm, the script that generates zsh.mdh doesn't know about #ifdef,
so it tries to export a variable that's declared conditionally in
mem.c.

All that's really necessary, though, is to make the typedef for
"Heapid" visible regardless of whether ZSH_HEAP_DEBUG is defined.

Index: Src/zsh.h
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/zsh.h,v
retrieving revision 1.175
diff -u -r1.175 zsh.h
--- Src/zsh.h   14 May 2011 00:07:42 -0000      1.175
+++ Src/zsh.h   14 May 2011 03:59:32 -0000
@@ -2327,7 +2327,6 @@
  * Memory management *
  *********************/
 
-#ifdef ZSH_HEAP_DEBUG
 /*
  * A Heapid is a type for identifying, uniquely up to the point where
  * the count of new identifiers wraps. all heaps that are or
@@ -2341,6 +2340,8 @@
  */
 typedef unsigned int Heapid;
 
+#ifdef ZSH_HEAP_DEBUG
+
 /* printf format specifier corresponding to Heapid */
 #define HEAPID_FMT     "%x"
 

-- 

Re: build problem with patch level 1.5301

[Peter Stephenson]

On Fri, 13 May 2011 21:00:00 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> All that's really necessary, though, is to make the typedef for
> "Heapid" visible regardless of whether ZSH_HEAP_DEBUG is defined.

No problem with that, but it's probably a bit safer also to hide the
prototypes of things we don't need to see.

Index: Src/mem.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/mem.c,v
retrieving revision 1.19
diff -p -u -r1.19 mem.c
--- Src/mem.c	14 May 2011 00:07:42 -0000	1.19
+++ Src/mem.c	14 May 2011 16:20:45 -0000
@@ -123,6 +123,7 @@ static Heap heaps;
 
 static Heap fheap;
 
+/**/
 #ifdef ZSH_HEAP_DEBUG
 /*
  * The heap ID we'll allocate next.
@@ -172,6 +173,8 @@ new_heap_id(void)
 {
     return next_heap_id++;
 }
+
+/**/
 #endif
 
 /* Use new heaps from now on. This returns the old heap-list. */
@@ -723,6 +726,7 @@ hrealloc(char *p, size_t old, size_t new
     }
 }
 
+/**/
 #ifdef ZSH_HEAP_DEBUG
 /*
  * Check if heap_id is the identifier of a currently valid heap,
@@ -768,6 +772,7 @@ memory_validate(Heapid heap_id)
 
     return 1;
 }
+/**/
 #endif
 
 /* allocate memory from the current memory pool and clear it */

-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/

Re: nits with new _git

[Nikolai Weibull]

On Sat, May 14, 2011 at 03:28, Mikael Magnusson <mikachu@gmail.com> wrote:
> Now that pws fixed the crash, I'm updating to the new _git, so far
> I've noticed these two things.
>
> git log --pret<tab> goes into correction, same for git show.
> % git log --pretty=
> ---- option
> --pretty  -- pretty print commit messages
> ---- corrections (errors: 2)
> --grep    -- limit commits to those with log messages matching the given pattern
> --pretty  -- pretty print commit messages
>
> Usually this would hint at some missing ret=0 somewhere, but I
> couldn't find any. Unconditionally returning 0 from _git does 'fix' it
> though, so it must be somewhere.

I can’t reproduce this issue.

> The other thing is that git branch -d -r <tab> doesn't complete remote branches.

Yes, this is still on the TODO list.

Re: nits with new _git

[Nikolai Weibull]

On Tue, May 17, 2011 at 14:56, Nikolai Weibull <now@bitwi.se> wrote:
> On Sat, May 14, 2011 at 03:28, Mikael Magnusson <mikachu@gmail.com> wrote:
>> Now that pws fixed the crash, I'm updating to the new _git, so far
>> I've noticed these two things.

>> The other thing is that git branch -d -r <tab> doesn't complete remote branches.

> Yes, this is still on the TODO list.

This has now been fixed in CVS.

Re: nits with new _git

[Mikael Magnusson]

On 17 May 2011 14:56, Nikolai Weibull <now@bitwi.se> wrote:
> On Sat, May 14, 2011 at 03:28, Mikael Magnusson <mikachu@gmail.com> wrote:
>> Now that pws fixed the crash, I'm updating to the new _git, so far
>> I've noticed these two things.
>>
>> git log --pret<tab> goes into correction, same for git show.
>> % git log --pretty=
>> ---- option
>> --pretty  -- pretty print commit messages
>> ---- corrections (errors: 2)
>> --grep    -- limit commits to those with log messages matching the given pattern
>> --pretty  -- pretty print commit messages
>>
>> Usually this would hint at some missing ret=0 somewhere, but I
>> couldn't find any. Unconditionally returning 0 from _git does 'fix' it
>> though, so it must be somewhere.
>
> I can’t reproduce this issue.

This was sort of a mindfuck, but I figured it out :). It looks like
this is a systematic error in the whole _git file, but should be
easily fixed. _git() itself defines local ret=1, then does
_call_function ret _git-$words[1], then _git-log() in this case sets
ret=0 on success. Sounds good? Not really, because it doesn't return
ret, so then _call_function overwrites ret with something else. The
solution would seem to simply not use _call_function since none of the
helper functions return anything, they just set ret directly.

--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -6023,11 +6023,11 @@ _git() {
       (option-or-argument)
         curcontext=${curcontext%:*:*}:git-$words[1]:

-        _call_function ret _git-$words[1]
+        _git-$words[1]
         ;;
     esac
   else
-    _call_function ret _$service
+    _$service
   fi
   return ret
 }



-- 
Mikael Magnusson

Re: nits with new _git

[Nikolai Weibull]

On Fri, Jun 3, 2011 at 23:39, Mikael Magnusson <mikachu@gmail.com> wrote:
> It looks like
> this is a systematic error in the whole _git file, but should be
> easily fixed. _git() itself defines local ret=1, then does
> _call_function ret _git-$words[1], then _git-log() in this case sets
> ret=0 on success. Sounds good? Not really, because it doesn't return
> ret, so then _call_function overwrites ret with something else. The
> solution would seem to simply not use _call_function since none of the
> helper functions return anything, they just set ret directly.

I’d rather remove all superfluous ⟨&& ret=0⟩s and add ⟨integer ret=1⟩
where appropriate.

Opinions?

Re: nits with new _git

[Nikolai Weibull]

On Wed, Jul 20, 2011 at 13:28, Nikolai Weibull <now@bitwi.se> wrote:
> On Fri, Jun 3, 2011 at 23:39, Mikael Magnusson <mikachu@gmail.com> wrote:
>> It looks like
>> this is a systematic error in the whole _git file, but should be
>> easily fixed. _git() itself defines local ret=1, then does
>> _call_function ret _git-$words[1], then _git-log() in this case sets
>> ret=0 on success. Sounds good? Not really, because it doesn't return
>> ret, so then _call_function overwrites ret with something else. The
>> solution would seem to simply not use _call_function since none of the
>> helper functions return anything, they just set ret directly.
>
> I’d rather remove all superfluous ⟨&& ret=0⟩s and add ⟨integer ret=1⟩
> where appropriate.
>
> Opinions?

I have now updated all functions to use the correct semantics.

Re: nits with new _git

[Mikael Magnusson]

On 21 July 2011 11:06, Nikolai Weibull <now@bitwi.se> wrote:
> On Wed, Jul 20, 2011 at 13:28, Nikolai Weibull <now@bitwi.se> wrote:
>> On Fri, Jun 3, 2011 at 23:39, Mikael Magnusson <mikachu@gmail.com> wrote:
>>> It looks like
>>> this is a systematic error in the whole _git file, but should be
>>> easily fixed. _git() itself defines local ret=1, then does
>>> _call_function ret _git-$words[1], then _git-log() in this case sets
>>> ret=0 on success. Sounds good? Not really, because it doesn't return
>>> ret, so then _call_function overwrites ret with something else. The
>>> solution would seem to simply not use _call_function since none of the
>>> helper functions return anything, they just set ret directly.
>>
>> I’d rather remove all superfluous ⟨&& ret=0⟩s and add ⟨integer ret=1⟩
>> where appropriate.
>>
>> Opinions?
>
> I have now updated all functions to use the correct semantics.

Ah, not much time to react, it seems to still work so I'm happy with
this way too. :)

-- 
Mikael Magnusson