zsh-workers
 help / color / mirror / code / Atom feed
* Updated _git completion (not attached)
@ 2011-01-24 20:34 Nikolai Weibull
  2011-02-19 13:39 ` Simon Ruderich
  2011-02-27  3:45 ` Wayne Davison
  0 siblings, 2 replies; 24+ messages in thread
From: Nikolai Weibull @ 2011-01-24 20:34 UTC (permalink / raw)
  To: Zsh Workers

OK.  I’m not doing this again.  This is perhaps the most boring thing
I’ve ever done (since writing _git the first time).  6037 lines of
pure boring.  167 remaining TODOs.

https://github.com/now/zsh

I don’t know where we go from here.  It’s a ridiculous history,
spanning almost one and a half years (though I took a rather long
hiatus during that time).  I reordered all functions to match the
order in the manual.  I reordered all option specifications to match
the order in the manual.  Perhaps the easiest thing to do is simply
throw away the whole history and simply overwrite the current _git
with this one.

I’d appreciate comments and updates to be provided as soon as
possible, so that I can review them while I still have most of this
internalized.

Anyway, this is for git (at or about) 1.7.3.5.

A note on the discussion of not using git ls-files for generating file
names together with _multi_parts: I tried the simple (e:…) filter for
_files but it turned out to be SLOW.  Perhaps someone else can look
into this and see if they can come up with a working solution.

I also updated the _git_commands function to use tags.  Can someone
please explain how I ignore plumbing-sync-helper-commands and
plumbing-internal-helper-commands?  I tried using the tag-order style,
but it wouldn’t take.  I’m obviously doing something wrong.  I also
removed support for user-commands here.  Perhaps they should be added
back in.


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Updated _git completion (not attached)
  2011-01-24 20:34 Updated _git completion (not attached) Nikolai Weibull
@ 2011-02-19 13:39 ` Simon Ruderich
  2011-02-21 14:34   ` Simon Ruderich
  2011-02-27  3:45 ` Wayne Davison
  1 sibling, 1 reply; 24+ messages in thread
From: Simon Ruderich @ 2011-02-19 13:39 UTC (permalink / raw)
  To: Nikolai Weibull; +Cc: Zsh Workers

[-- Attachment #1: Type: text/plain, Size: 889 bytes --]

On Mon, Jan 24, 2011 at 09:34:57PM +0100, Nikolai Weibull wrote:
> I’d appreciate comments and updates to be provided as soon as
> possible, so that I can review them while I still have most of this
> internalized.

First thanks for your great work - Git's completion is awesome
and very useful to me.

> Anyway, this is for git (at or about) 1.7.3.5.

Works fine for me, but I have some minor issues:

- It seems like git checkout doesn't list only modified files but
  all files. IIRC this was handled better in the old completion,
  only offering modified files.
- git log -p <tab> doesn't list any files, without the -p it
  works fine.

But besides these minor issues the new Git completion works
great, thank you very much for your great work.

Regards,
Simon
-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Updated _git completion (not attached)
  2011-02-19 13:39 ` Simon Ruderich
@ 2011-02-21 14:34   ` Simon Ruderich
  0 siblings, 0 replies; 24+ messages in thread
From: Simon Ruderich @ 2011-02-21 14:34 UTC (permalink / raw)
  To: Nikolai Weibull; +Cc: Zsh Workers

[-- Attachment #1: Type: text/plain, Size: 588 bytes --]

On Sat, Feb 19, 2011 at 02:39:57PM +0100, Simon Ruderich wrote:
> [snip]
>
> Works fine for me, but I have some minor issues:
>
> - It seems like git checkout doesn't list only modified files but
>   all files. IIRC this was handled better in the old completion,
>   only offering modified files.
> - git log -p <tab> doesn't list any files, without the -p it
>   works fine.

- Same problem with git add -p <tab>.

Not sure if it applies to more commands.

Regards,
Simon
-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Updated _git completion (not attached)
  2011-01-24 20:34 Updated _git completion (not attached) Nikolai Weibull
  2011-02-19 13:39 ` Simon Ruderich
@ 2011-02-27  3:45 ` Wayne Davison
  2011-02-27  7:47   ` Frank Terbeck
  1 sibling, 1 reply; 24+ messages in thread
From: Wayne Davison @ 2011-02-27  3:45 UTC (permalink / raw)
  To: Zsh Workers

[-- Attachment #1: Type: text/plain, Size: 326 bytes --]

>
> https://github.com/now/zsh


So, is anyone evaluating this for inclusion?  I gave it a look, but I
haven't done enough completion-function work to really evaluate if we want
to do any merging or just replace what we have now with the reworked
version.  I'd be glad to check it in, if that's what we want to do.

..wayne..

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Updated _git completion (not attached)
  2011-02-27  3:45 ` Wayne Davison
@ 2011-02-27  7:47   ` Frank Terbeck
  2011-02-27 12:03     ` Nikolai Weibull
  0 siblings, 1 reply; 24+ messages in thread
From: Frank Terbeck @ 2011-02-27  7:47 UTC (permalink / raw)
  To: Wayne Davison; +Cc: Zsh Workers

Wayne Davison wrote:
>> https://github.com/now/zsh
> So, is anyone evaluating this for inclusion?  I gave it a look, but I
> haven't done enough completion-function work to really evaluate if we want
> to do any merging or just replace what we have now with the reworked
> version.  I'd be glad to check it in, if that's what we want to do.

Since Nikolai (the guy who wrote that updated function) wrote most of
what we have in CVS right now, I think we should merge that new _git
completion and see if it breaks for anyone.  I suspect it won't.

Regards, Frank

-- 
In protocol design, perfection has been reached not when there is
nothing left to add, but when there is nothing left to take away.
                                                  -- RFC 1925


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Updated _git completion (not attached)
  2011-02-27  7:47   ` Frank Terbeck
@ 2011-02-27 12:03     ` Nikolai Weibull
  2011-02-27 12:25       ` Nikolai Weibull
  0 siblings, 1 reply; 24+ messages in thread
From: Nikolai Weibull @ 2011-02-27 12:03 UTC (permalink / raw)
  To: Frank Terbeck; +Cc: Wayne Davison, Zsh Workers

On Sun, Feb 27, 2011 at 08:47, Frank Terbeck <ft@bewatermyfriend.org> wrote:
> Wayne Davison wrote:
>>> https://github.com/now/zsh
>> So, is anyone evaluating this for inclusion?  I gave it a look, but I
>> haven't done enough completion-function work to really evaluate if we want
>> to do any merging or just replace what we have now with the reworked
>> version.  I'd be glad to check it in, if that's what we want to do.
>
> Since Nikolai (the guy who wrote that updated function) wrote most of
> what we have in CVS right now, I think we should merge that new _git
> completion and see if it breaks for anyone.  I suspect it won't.

I was kind of hoping that people would check out my updated version,
give it a test, give feedback, and then we’d merge, but I have only
gotten two complaints (from Simon).  I would like to fix them first,
then we can merge and let more problems be reported.

I will look into the problems Simon has reported so far today.


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Updated _git completion (not attached)
  2011-02-27 12:03     ` Nikolai Weibull
@ 2011-02-27 12:25       ` Nikolai Weibull
  2011-02-28 13:58         ` Simon Ruderich
                           ` (2 more replies)
  0 siblings, 3 replies; 24+ messages in thread
From: Nikolai Weibull @ 2011-02-27 12:25 UTC (permalink / raw)
  To: Frank Terbeck; +Cc: Wayne Davison, Zsh Workers

On Sun, Feb 27, 2011 at 13:03, Nikolai Weibull <now@bitwi.se> wrote:
> On Sun, Feb 27, 2011 at 08:47, Frank Terbeck <ft@bewatermyfriend.org> wrote:
>> Wayne Davison wrote:
>>>> https://github.com/now/zsh
>>> So, is anyone evaluating this for inclusion?  I gave it a look, but I
>>> haven't done enough completion-function work to really evaluate if we want
>>> to do any merging or just replace what we have now with the reworked
>>> version.  I'd be glad to check it in, if that's what we want to do.
>>
>> Since Nikolai (the guy who wrote that updated function) wrote most of
>> what we have in CVS right now, I think we should merge that new _git
>> completion and see if it breaks for anyone.  I suspect it won't.
>
> I was kind of hoping that people would check out my updated version,
> give it a test, give feedback, and then we’d merge, but I have only
> gotten two complaints (from Simon).  I would like to fix them first,
> then we can merge and let more problems be reported.
>
> I will look into the problems Simon has reported so far today.

It was an easy fix.  It’s now available in the repository mentioned above.

Wayne, I would appreciate it if you merge this branch into the main repository.

Thanks!


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Updated _git completion (not attached)
  2011-02-27 12:25       ` Nikolai Weibull
@ 2011-02-28 13:58         ` Simon Ruderich
  2011-02-28 16:33           ` Wayne Davison
  2011-02-28 20:58         ` Frank Terbeck
  2011-03-04 12:57         ` Frank Terbeck
  2 siblings, 1 reply; 24+ messages in thread
From: Simon Ruderich @ 2011-02-28 13:58 UTC (permalink / raw)
  To: Zsh Workers

[-- Attachment #1: Type: text/plain, Size: 832 bytes --]

On Sat, Feb 19, 2011 at 02:39:57PM +0100, Simon Ruderich wrote:
> [snip]
>
> - It seems like git checkout doesn't list only modified files but
>   all files. IIRC this was handled better in the old completion,
>   only offering modified files.

Would it be difficult to add support for this? I just checked and
I was wrong, it wasn't there, but it would be great if this could
get added - similar to git add's completion.

> - git log -p <tab> doesn't list any files, without the -p it
>   works fine.

On Sun, Feb 27, 2011 at 01:25:50PM +0100, Nikolai Weibull wrote:
> [snip]
>
> It was an easy fix.  It’s now available in the repository mentioned above.

Thank you for the fix, works great.

Regards,
Simon
-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Updated _git completion (not attached)
  2011-02-28 13:58         ` Simon Ruderich
@ 2011-02-28 16:33           ` Wayne Davison
  2011-02-28 17:07             ` Nikolai Weibull
  0 siblings, 1 reply; 24+ messages in thread
From: Wayne Davison @ 2011-02-28 16:33 UTC (permalink / raw)
  To: Simon Ruderich; +Cc: Zsh Workers

[-- Attachment #1: Type: text/plain, Size: 962 bytes --]

>
> On Sat, Feb 19, 2011 at 02:39:57PM +0100, Simon Ruderich wrote:
> - It seems like git checkout doesn't list only modified files but all
> files.
>

One thing to keep in mind is that it is right for it to list all
(known-to-git) files after a tag or hash, since the user may want to change
the file to a prior state.  It is only the default "git checkout FILE" that
only really has meaning for changed files.  And, I had thought for sure that
git didn't allow filenames after git checkout -f (and indeed, the current
completion code doesn't complete files after it), but in my testing of git
1.7.1, it accepted files to force w/o complaint.

For instance, "git checkout -f zsh-4.3.9 ChangeLog" changes to that version,
and if the file is tweaked and followed by "git checkout -f ChangeLog", the
zsh-4.3.9 content is restored.  In both cases it seems to me that the -f was
superfluous, but allowed (since the commands do the same thing w/o the -f).

..wayne..

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Updated _git completion (not attached)
  2011-02-28 16:33           ` Wayne Davison
@ 2011-02-28 17:07             ` Nikolai Weibull
  2011-02-28 17:21               ` Mikael Magnusson
  2011-02-28 20:32               ` Simon Ruderich
  0 siblings, 2 replies; 24+ messages in thread
From: Nikolai Weibull @ 2011-02-28 17:07 UTC (permalink / raw)
  To: Wayne Davison; +Cc: Simon Ruderich, Zsh Workers

On Mon, Feb 28, 2011 at 17:33, Wayne Davison
<wayned@users.sourceforge.net> wrote:
>>
>> On Sat, Feb 19, 2011 at 02:39:57PM +0100, Simon Ruderich wrote:
>> - It seems like git checkout doesn't list only modified files but all
>> files.

> One thing to keep in mind is that it is right for it to list all
> (known-to-git) files after a tag or hash, since the user may want to change
> the file to a prior state.  It is only the default "git checkout FILE" that
> only really has meaning for changed files.

> And, I had thought for sure that
> git didn't allow filenames after git checkout -f (and indeed, the current
> completion code doesn't complete files after it), but in my testing of git
> 1.7.1, it accepted files to force w/o complaint.

If no tree-ish is given it uses HEAD as the tree-ish to look for files
in.  Perhaps it should only list files modified between HEAD and the
working tree when the tree-ish is HEAD.

> For instance, "git checkout -f zsh-4.3.9 ChangeLog" changes to that version,
> and if the file is tweaked and followed by "git checkout -f ChangeLog", the
> zsh-4.3.9 content is restored.  In both cases it seems to me that the -f was
> superfluous, but allowed (since the commands do the same thing w/o the -f).

I don’t see how -f should change anything in the completion.


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Updated _git completion (not attached)
  2011-02-28 17:07             ` Nikolai Weibull
@ 2011-02-28 17:21               ` Mikael Magnusson
  2011-02-28 20:32               ` Simon Ruderich
  1 sibling, 0 replies; 24+ messages in thread
From: Mikael Magnusson @ 2011-02-28 17:21 UTC (permalink / raw)
  To: Nikolai Weibull; +Cc: Wayne Davison, Simon Ruderich, Zsh Workers

On 28 February 2011 18:07, Nikolai Weibull <now@bitwi.se> wrote:
> On Mon, Feb 28, 2011 at 17:33, Wayne Davison
> <wayned@users.sourceforge.net> wrote:
>>>
>>> On Sat, Feb 19, 2011 at 02:39:57PM +0100, Simon Ruderich wrote:
>>> - It seems like git checkout doesn't list only modified files but all
>>> files.
>
>> One thing to keep in mind is that it is right for it to list all
>> (known-to-git) files after a tag or hash, since the user may want to change
>> the file to a prior state.  It is only the default "git checkout FILE" that
>> only really has meaning for changed files.
>
>> And, I had thought for sure that
>> git didn't allow filenames after git checkout -f (and indeed, the current
>> completion code doesn't complete files after it), but in my testing of git
>> 1.7.1, it accepted files to force w/o complaint.
>
> If no tree-ish is given it uses HEAD as the tree-ish to look for files
> in.  Perhaps it should only list files modified between HEAD and the
> working tree when the tree-ish is HEAD.

Actually it looks in the index when no tree-ish is given, which is why
it restored the previously specified zsh-4.3.9 version of ChangeLog
when no tree-ish was given; that version was still in the index. If
you want to restore the current branch's version of a file when it has
changed staged, you have to explicitly say HEAD.

(This also applies when you have done changes to a file, git-added
them, and then do more changes. git checkout with no tree-ish and the
filename will then restore the version you last added.)

As far as I can tell, -f only affects behaviour when switching
branches or when specified files have unmerged changes (as in from a
merge or rebase conflict), and not otherwise when specifying explicit
files even when they have unstaged changes.

-- 
Mikael Magnusson


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Updated _git completion (not attached)
  2011-02-28 17:07             ` Nikolai Weibull
  2011-02-28 17:21               ` Mikael Magnusson
@ 2011-02-28 20:32               ` Simon Ruderich
  1 sibling, 0 replies; 24+ messages in thread
From: Simon Ruderich @ 2011-02-28 20:32 UTC (permalink / raw)
  To: zsh-workers

[-- Attachment #1: Type: text/plain, Size: 476 bytes --]

On Mon, Feb 28, 2011 at 06:07:15PM +0100, Nikolai Weibull wrote:
> If no tree-ish is given it uses HEAD as the tree-ish to look for files
> in.  Perhaps it should only list files modified between HEAD and the
> working tree when the tree-ish is HEAD.

That would be great. I often use git checkout to remove changes
and that would make it even faster.

Regards,
Simon
-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Updated _git completion (not attached)
  2011-02-27 12:25       ` Nikolai Weibull
  2011-02-28 13:58         ` Simon Ruderich
@ 2011-02-28 20:58         ` Frank Terbeck
  2011-03-04 12:57         ` Frank Terbeck
  2 siblings, 0 replies; 24+ messages in thread
From: Frank Terbeck @ 2011-02-28 20:58 UTC (permalink / raw)
  To: Nikolai Weibull; +Cc: Zsh Workers

Nikolai Weibull wrote:
> On Sun, Feb 27, 2011 at 13:03, Nikolai Weibull <now@bitwi.se> wrote:
[...]
>> I was kind of hoping that people would check out my updated version,
>> give it a test, give feedback [...]
> Wayne, I would appreciate it if you merge this branch into the main repository.

Here's one: When using the git-branch completion, the `-d' option (`-D'
likewise) offers the `-r' option. But then doing:

zsh% git branch -D -r <tab>

does not offer remote branches but just local ones.

Also when reversing the options:

zsh% git branch -r -<tab>

doesn't offer `-d' or `-D' and

zsh% git branch -r -d <tab>

only offers a few more options, but no non-option parameters at all.

Regards, Frank

-- 
In protocol design, perfection has been reached not when there is
nothing left to add, but when there is nothing left to take away.
                                                  -- RFC 1925


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Updated _git completion (not attached)
  2011-02-27 12:25       ` Nikolai Weibull
  2011-02-28 13:58         ` Simon Ruderich
  2011-02-28 20:58         ` Frank Terbeck
@ 2011-03-04 12:57         ` Frank Terbeck
  2011-03-18 20:34           ` Nikolai Weibull
  2 siblings, 1 reply; 24+ messages in thread
From: Frank Terbeck @ 2011-03-04 12:57 UTC (permalink / raw)
  To: Nikolai Weibull; +Cc: Zsh Workers

More feedback:

% git send-email --suppress-from --to=zsh-workers@zsh.org 000<TAB>
_arguments:comparguments:312: invalid option definition: --smtp-user=[specify user to use for SMTP-AUTH:smtp user:_users
_arguments:comparguments:312: invalid option definition: --smtp-user=[specify user to use for SMTP-AUTH:smtp user:_users

Regards, Frank


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Updated _git completion (not attached)
  2011-03-04 12:57         ` Frank Terbeck
@ 2011-03-18 20:34           ` Nikolai Weibull
  2011-03-18 20:43             ` Frank Terbeck
  0 siblings, 1 reply; 24+ messages in thread
From: Nikolai Weibull @ 2011-03-18 20:34 UTC (permalink / raw)
  To: Frank Terbeck; +Cc: Zsh Workers

On Fri, Mar 4, 2011 at 13:57, Frank Terbeck <ft@bewatermyfriend.org> wrote:
> More feedback:
>
> % git send-email --suppress-from --to=zsh-workers@zsh.org 000<TAB>
> _arguments:comparguments:312: invalid option definition: --smtp-user=[specify user to use for SMTP-AUTH:smtp user:_users
> _arguments:comparguments:312: invalid option definition: --smtp-user=[specify user to use for SMTP-AUTH:smtp user:_users

I would like to fix the bugs mentioned in this thread now.  Should I
do future work against CVS?  Is there a Git mirror that I can use?


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Updated _git completion (not attached)
  2011-03-18 20:34           ` Nikolai Weibull
@ 2011-03-18 20:43             ` Frank Terbeck
  2011-03-18 21:09               ` Nikolai Weibull
  0 siblings, 1 reply; 24+ messages in thread
From: Frank Terbeck @ 2011-03-18 20:43 UTC (permalink / raw)
  To: Nikolai Weibull; +Cc: Zsh Workers

Nikolai Weibull wrote:
> On Fri, Mar 4, 2011 at 13:57, Frank Terbeck <ft@bewatermyfriend.org> wrote:
>> More feedback:
>>
>> % git send-email --suppress-from --to=zsh-workers@zsh.org 000<TAB>
>> _arguments:comparguments:312: invalid option definition: --smtp-user=[specify user to use for SMTP-AUTH:smtp user:_users
>> _arguments:comparguments:312: invalid option definition: --smtp-user=[specify user to use for SMTP-AUTH:smtp user:_users
>
> I would like to fix the bugs mentioned in this thread now.  Should I
> do future work against CVS?  Is there a Git mirror that I can use?

Yes, there's a mirror at:

     git://zsh.git.sf.net/gitroot/zsh/zsh

It's updated every 10 minutes or so. So, the difference between it and
the official CVS repository is negligible.

Regards, Frank

-- 
In protocol design, perfection has been reached not when there is
nothing left to add, but when there is nothing left to take away.
                                                  -- RFC 1925


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Updated _git completion (not attached)
  2011-03-18 20:43             ` Frank Terbeck
@ 2011-03-18 21:09               ` Nikolai Weibull
  2011-03-18 21:18                 ` Frank Terbeck
  0 siblings, 1 reply; 24+ messages in thread
From: Nikolai Weibull @ 2011-03-18 21:09 UTC (permalink / raw)
  To: Frank Terbeck; +Cc: Zsh Workers

On Fri, Mar 18, 2011 at 21:43, Frank Terbeck <ft@bewatermyfriend.org> wrote:
> Nikolai Weibull wrote:
>> On Fri, Mar 4, 2011 at 13:57, Frank Terbeck <ft@bewatermyfriend.org> wrote:
>>> More feedback:
>>>
>>> % git send-email --suppress-from --to=zsh-workers@zsh.org 000<TAB>
>>> _arguments:comparguments:312: invalid option definition: --smtp-user=[specify user to use for SMTP-AUTH:smtp user:_users
>>> _arguments:comparguments:312: invalid option definition: --smtp-user=[specify user to use for SMTP-AUTH:smtp user:_users
>>
>> I would like to fix the bugs mentioned in this thread now.  Should I
>> do future work against CVS?  Is there a Git mirror that I can use?

> Yes, there's a mirror at:
>
>     git://zsh.git.sf.net/gitroot/zsh/zsh
>
> It's updated every 10 minutes or so. So, the difference between it and
> the official CVS repository is negligible.

OK, I’ve fixed the bugs and suggestions made in this thread.  How do
you (that is, the Zsh maintainers) want them submitted?  As a patch
series to this list?


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Updated _git completion (not attached)
  2011-03-18 21:09               ` Nikolai Weibull
@ 2011-03-18 21:18                 ` Frank Terbeck
  2011-03-20  2:24                   ` Johan Sundström
  0 siblings, 1 reply; 24+ messages in thread
From: Frank Terbeck @ 2011-03-18 21:18 UTC (permalink / raw)
  To: Nikolai Weibull; +Cc: Zsh Workers

Nikolai Weibull wrote:
[...]
> OK, I’ve fixed the bugs and suggestions made in this thread.  How do
> you (that is, the Zsh maintainers) want them submitted?  As a patch
> series to this list?

This document details how patches should be submitted:

<http://zsh.git.sourceforge.net/git/gitweb.cgi?p=zsh/zsh;a=blob_plain;f=Etc/zsh-development-guide;hb=HEAD>

That being said, the output of "git format-patch" is usable,
too. Exchanging git's default "[PATCH]" by "PATCH: " would be extra
sugar.

Regards, Frank

-- 
In protocol design, perfection has been reached not when there is
nothing left to add, but when there is nothing left to take away.
                                                  -- RFC 1925


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Updated _git completion (not attached)
  2011-03-18 21:18                 ` Frank Terbeck
@ 2011-03-20  2:24                   ` Johan Sundström
  2011-03-20  8:05                     ` Submitting patches [was: Re: Updated _git completion (not attached)] Frank Terbeck
  0 siblings, 1 reply; 24+ messages in thread
From: Johan Sundström @ 2011-03-20  2:24 UTC (permalink / raw)
  To: Frank Terbeck; +Cc: Nikolai Weibull, Zsh Workers

[-- Attachment #1: Type: text/plain, Size: 1050 bytes --]

On Fri, Mar 18, 2011 at 14:18, Frank Terbeck <ft@bewatermyfriend.org> wrote:

> Nikolai Weibull wrote:
> [...]
> > OK, I’ve fixed the bugs and suggestions made in this thread.  How do
> > you (that is, the Zsh maintainers) want them submitted?  As a patch
> > series to this list?
>
> This document details how patches should be submitted:
>
> <
> http://zsh.git.sourceforge.net/git/gitweb.cgi?p=zsh/zsh;a=blob_plain;f=Etc/zsh-development-guide;hb=HEAD
> >
>

This document doesn't mention it yet, but I assume it's best to submit
patches in the message body rather than as attachments? (Unless, I suppose,
they contain binary content.)

That being said, the output of "git format-patch" is usable,
> too. Exchanging git's default "[PATCH]" by "PATCH: " would be extra
> sugar.
>

Running perl -pi -e 's/^(Subject: ).(PATCH[^]]*)./$1$2:/' *.patch on
format-patch files fixes that (it seems command line options to git can't).
Maybe a useful note for the dev guide?

-- 
 / Johan Sundström, http://ecmanaut.blogspot.com/

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Submitting patches [was: Re: Updated _git completion (not attached)]
  2011-03-20  2:24                   ` Johan Sundström
@ 2011-03-20  8:05                     ` Frank Terbeck
  2011-03-20  8:47                       ` Bart Schaefer
  2011-03-20 15:34                       ` Benjamin R. Haskell
  0 siblings, 2 replies; 24+ messages in thread
From: Frank Terbeck @ 2011-03-20  8:05 UTC (permalink / raw)
  To: Johan Sundström; +Cc: Nikolai Weibull, Zsh Workers

Johan Sundström wrote:
> On Fri, Mar 18, 2011 at 14:18, Frank Terbeck <ft@bewatermyfriend.org> wrote:
>> [Etc/zsh-development-guide]
>
> This document doesn't mention it yet, but I assume it's best to submit
> patches in the message body rather than as attachments? (Unless, I suppose,
> they contain binary content.)

I think this is true (simply because it makes commenting patches
easier). But my answer on the matter is certainly not authoritative. I
thought I had seen similar comments on the list before; but I couldn't
find any in a quick search via
<http://www.zsh.org/cgi-bin/mla/wilma/workers>.

>> That being said, the output of "git format-patch" is usable,
>> too. Exchanging git's default "[PATCH]" by "PATCH: " would be extra
>> sugar.
>
> Running perl -pi -e 's/^(Subject: ).(PATCH[^]]*)./$1$2:/' *.patch on
> format-patch files fixes that (it seems command line options to git can't).
> Maybe a useful note for the dev guide?

The archive reacts to anything that starts with "PATCH". You can see the
results on <http://www.zsh.org/mla/patches.shtml>. So, your Perl snippet
would work. Even for numbered patches. The only thing I could think of
this could break is if there's a second line in the file that matches
your regular expression. Here is a zsh function which is using ed(1)
instead of Perl to achieve something similar. This is pretty much what
I've been using in the past.


function gsfix() {
    # zsh-workers prefers PATCH: as its prefix.
    #   fixes [PATCH] to PATCH:
    #     and [PATCH m/n] to PATCH: (m/n)

    if (( ${#argv} == 0 )); then
        printf 'Usage: gsfix <FILE(s)>\n'
        return 1
    fi

    local file
    for file in "$@"; do
        if [[ ! -w "${file}" ]]; then
            printf 'Cannot write to file: %s. Skipping.\n' "${file}"
            continue
        fi
        (
            printf '/^Subject: \[\n'
            printf 's,\[PATCH \([0-9]\+/[0-9]\+\)\],PATCH: (\\1),\n'
            printf 'w\nq\n'
        ) | ed "${file}" > /dev/null 2>&1

        [ "$?" -eq 0 ] && continue

        (
            printf '/^Subject: \[\n'
            printf 's,\[PATCH\],PATCH:,\n'
            printf 'w\nq\n'
        ) | ed "${file}" > /dev/null 2>&1
    done

    return 0
}

I don't know if we should put either into the development guide as a
note, since git isn't zsh's official VCS. And if it were, we could
probably change the patch archive code to handle git's "[PATCH] "
prefix.

Regards, Frank

-- 
In protocol design, perfection has been reached not when there is
nothing left to add, but when there is nothing left to take away.
                                                  -- RFC 1925


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Submitting patches [was: Re: Updated _git completion (not attached)]
  2011-03-20  8:05                     ` Submitting patches [was: Re: Updated _git completion (not attached)] Frank Terbeck
@ 2011-03-20  8:47                       ` Bart Schaefer
  2011-03-20 15:41                         ` Benjamin R. Haskell
  2011-03-20 15:34                       ` Benjamin R. Haskell
  1 sibling, 1 reply; 24+ messages in thread
From: Bart Schaefer @ 2011-03-20  8:47 UTC (permalink / raw)
  To: Zsh Workers

On Mar 20,  9:05am, Frank Terbeck wrote:
} Subject: Submitting patches [was: Re: Updated _git completion (not attache
}
} Johan Sundstrom wrote:
} > On Fri, Mar 18, 2011 at 14:18, Frank Terbeck <ft@bewatermyfriend.org> wrote:
} >> [Etc/zsh-development-guide]
} >
} > This document doesn't mention it yet, but I assume it's best to
} > submit patches in the message body rather than as attachments?
} > (Unless, I suppose, they contain binary content.)
} 
} I think this is true (simply because it makes commenting patches
} easier). But my answer on the matter is certainly not authoritative. I
} thought I had seen similar comments on the list before; but I couldn't
} find any in a quick search via
} <http://www.zsh.org/cgi-bin/mla/wilma/workers>.

There's a preference for patches in the message body, yes.  This is less
important than it used to be because the archive software has gotten
better at inlining text parts.

One my own pet peeves is the wild inconsistency of mime-type labeling of
attached diffs depending on what email client is used to attach them.
text/x-diff, text/x-patch, application/octet-stream, etc. etc.  No,
dammit, they're text/plain.  Just say so.

-- 


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Submitting patches [was: Re: Updated _git completion (not attached)]
  2011-03-20  8:05                     ` Submitting patches [was: Re: Updated _git completion (not attached)] Frank Terbeck
  2011-03-20  8:47                       ` Bart Schaefer
@ 2011-03-20 15:34                       ` Benjamin R. Haskell
  1 sibling, 0 replies; 24+ messages in thread
From: Benjamin R. Haskell @ 2011-03-20 15:34 UTC (permalink / raw)
  To: Frank Terbeck; +Cc: Johan Sundström, Nikolai Weibull, Zsh Workers

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2663 bytes --]

On Sun, 20 Mar 2011, Frank Terbeck wrote:

> Johan Sundström wrote:
>> On Fri, Mar 18, 2011 at 14:18, Frank Terbeck <ft@bewatermyfriend.org> wrote:
>>> [Etc/zsh-development-guide]
>>
>>> That being said, the output of "git format-patch" is usable, too. 
>>> Exchanging git's default "[PATCH]" by "PATCH: " would be extra 
>>> sugar.
>>
>> Running perl -pi -e 's/^(Subject: ).(PATCH[^]]*)./$1$2:/' *.patch on 
>> format-patch files fixes that (it seems command line options to git 
>> can't).  Maybe a useful note for the dev guide?
>
> The archive reacts to anything that starts with "PATCH". You can see the
> results on <http://www.zsh.org/mla/patches.shtml>. So, your Perl snippet
> would work. Even for numbered patches.

Based on that list, it appears that no Perl script is necessary. 
Nikolai's patches labeled with the standard `git` way were picked up 
well enough:

Subject: [PATCH 3/4] Fix git-branch -[dD] -r completion
Subject: [PATCH 1/4] Fix typo
Subject: [PATCH 4/4] Fix typo in git-send-email completion
Subject: [PATCH 2/4] Only show modified files for git-checkout without tree

were grabbed as:

3/4] Fix git-branch -[dD] -r completion
1/4] Fix typo
4/4] Fix typo in git-send-email completion
2/4] Only show modified files for git-checkout without tree

Personally, I think it's better that the "series" info stays there.  The 
only ugliness is the '] ' instead of ': '.

And, with `git send-email --annotate` (which IMO is easier than `git 
format-patch` and then firing up an email client), there's no clear 
point at which to run the Perl script.  (Though, with --annotate, you 
can simply modify the Subject: line in your editor.)

I have the following in my .gitconfig:

[alias]
 	# ... 45 lines later ...
 	email-zsh = "!email () { email=\"$(git config --get user.email)\" ; user=\"$(git config --get user.name)\" ; git send-email --annotate --envelope-sender='<'$email'>' --from=\"$user <$email>\" --cc=\"$user <$email>\" --to='Zsh Workers <zsh-workers@zsh.org>' \"$@\" ; } ; email"

--annotate - allows editing the messages
--envelope-sender - helps with some mailing lists
--cc - so I get a copy of the message
--from - (not usually necessary, I don't think -- remnant from doing 
other email cartwheels in other aliases)

The "!fn-name () { function-body ; } ; fn-name" idiom might not be 
necessary for this case, but it comes in handy when you want to munge 
the arguments list inside function-body.

The other options should be obvious, and any other options you pass on 
the commandline (--dry-run, list of revisions, etc.) get passed up to 
send-email, which passes any options it doesn't use up to format-patch.

-- 
Best,
Ben

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Submitting patches [was: Re: Updated _git completion (not attached)]
  2011-03-20  8:47                       ` Bart Schaefer
@ 2011-03-20 15:41                         ` Benjamin R. Haskell
  2011-03-20 17:50                           ` Bart Schaefer
  0 siblings, 1 reply; 24+ messages in thread
From: Benjamin R. Haskell @ 2011-03-20 15:41 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh Workers

On Sun, 20 Mar 2011, Bart Schaefer wrote:

> On Mar 20,  9:05am, Frank Terbeck wrote:
> } Subject: Submitting patches [was: Re: Updated _git completion (not attache
> }
> } Johan Sundstrom wrote:
> } > On Fri, Mar 18, 2011 at 14:18, Frank Terbeck wrote:
> } >> [Etc/zsh-development-guide]
> } >
> } > This document doesn't mention it yet, but I assume it's best to } 
> > submit patches in the message body rather than as attachments?  } > 
> > (Unless, I suppose, they contain binary content.)
> }
> } I think this is true (simply because it makes commenting patches 
> } easier). But my answer on the matter is certainly not authoritative. I 
> } thought I had seen similar comments on the list before; but I couldn't 
> } find any in a quick search via
> } <http://www.zsh.org/cgi-bin/mla/wilma/workers>.
>
> There's a preference for patches in the message body, yes.  This is 
> less important than it used to be because the archive software has 
> gotten better at inlining text parts.
>
> One my own pet peeves is the wild inconsistency of mime-type labeling 
> of attached diffs depending on what email client is used to attach 
> them.  text/x-diff, text/x-patch, application/octet-stream, etc. etc. 
> No, dammit, they're text/plain.  Just say so.

Patches are text/* ...except when they're not.

Working on a daily basis with a codebase containing inconsistent line 
endings (CRLF and LF), I see the rationale.  application/octet-stream is 
my preference in that case.  It allows the Windows devs to cleanly apply 
my patches and vice versa.  And since it works fine in the 
consistent-line-endings case, there's no reason to use two different 
methods.  (I do, though, since I use `git` whenever possible; `git svn` 
at work, where I deal with the aforementioned madness.)

-- 
Best,
Ben


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Submitting patches [was: Re: Updated _git completion (not attached)]
  2011-03-20 15:41                         ` Benjamin R. Haskell
@ 2011-03-20 17:50                           ` Bart Schaefer
  0 siblings, 0 replies; 24+ messages in thread
From: Bart Schaefer @ 2011-03-20 17:50 UTC (permalink / raw)
  To: Zsh Workers

On Mar 20, 11:41am, Benjamin R. Haskell wrote:
}
} Patches are text/* ...except when they're not.

Reasonable position in the general case, but if someone is submitting
non-text patches for zsh we have a larger issue.

(I'll revise that sentiment when someone volunteers to translate the
doc into non-English languages, but until then ...)


^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2011-03-20 17:50 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-24 20:34 Updated _git completion (not attached) Nikolai Weibull
2011-02-19 13:39 ` Simon Ruderich
2011-02-21 14:34   ` Simon Ruderich
2011-02-27  3:45 ` Wayne Davison
2011-02-27  7:47   ` Frank Terbeck
2011-02-27 12:03     ` Nikolai Weibull
2011-02-27 12:25       ` Nikolai Weibull
2011-02-28 13:58         ` Simon Ruderich
2011-02-28 16:33           ` Wayne Davison
2011-02-28 17:07             ` Nikolai Weibull
2011-02-28 17:21               ` Mikael Magnusson
2011-02-28 20:32               ` Simon Ruderich
2011-02-28 20:58         ` Frank Terbeck
2011-03-04 12:57         ` Frank Terbeck
2011-03-18 20:34           ` Nikolai Weibull
2011-03-18 20:43             ` Frank Terbeck
2011-03-18 21:09               ` Nikolai Weibull
2011-03-18 21:18                 ` Frank Terbeck
2011-03-20  2:24                   ` Johan Sundström
2011-03-20  8:05                     ` Submitting patches [was: Re: Updated _git completion (not attached)] Frank Terbeck
2011-03-20  8:47                       ` Bart Schaefer
2011-03-20 15:41                         ` Benjamin R. Haskell
2011-03-20 17:50                           ` Bart Schaefer
2011-03-20 15:34                       ` Benjamin R. Haskell

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).