threads / patch / 10090

patchSupport tags in uncommit - use git_id instead of rev_parse

Subject: [PATCH] Support tags in uncommit - use git_id instead of rev_parse

## tl;dr

7 messages between Sep 30, 2007 and Oct 7, 2007. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

Pavel Roskin· Sep 30, 2007, 17:26 UTC · lore
Signed-off-by: Pavel Roskin <proski@gnu.org>
---
 stgit/commands/uncommit.py |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to stgit/commands/uncommit.py +1 −1
diff --git a/stgit/commands/uncommit.py b/stgit/commands/uncommit.py
index 0cd0fb0..c22d3ea 100644
--- a/stgit/commands/uncommit.py
+++ b/stgit/commands/uncommit.py
@@ -65,7 +65,7 @@ def func(parser, options, args):
         if len(args) != 0:
             parser.error('cannot specify patch name with --to')
         patch_nr = patchnames = None
-        to_commit = git.rev_parse(options.to)
+        to_commit = git_id(options.to)
     elif options.number:
         if options.number <= 0:
             parser.error('invalid value passed to --number')
Catalin Marinas· Oct 1, 2007, 22:00 UTC · re: Pavel Roskin · lore

Re: [PATCH] Support tags in uncommit - use git_id instead of rev_parse

On 30/09/2007, Pavel Roskin <proski@gnu.org> wrote:
> Signed-off-by: Pavel Roskin <proski@gnu.org>

With this patch, uncommit can take patch names (with modifiers) as the --to argument. When would this be needed?

To allow tags, maybe just pass something like "git.rev_parse(options.to + '^{commit}')" or just modify git.rev_parse to do it (and git_id to avoid it).

-- 
Catalin
Pavel Roskin· Oct 2, 2007, 22:03 UTC · re: Catalin Marinas · lore

Re: [PATCH] Support tags in uncommit - use git_id instead of rev_parse

On Mon, 2007-10-01 at 23:00 +0100, Catalin Marinas wrote:
Show 5 quoted lines
> On 30/09/2007, Pavel Roskin <proski@gnu.org> wrote:
> > Signed-off-by: Pavel Roskin <proski@gnu.org>
> 
> With this patch, uncommit can take patch names (with modifiers) as the
> --to argument. When would this be needed?
Probably never.
> To allow tags, maybe just pass something like
> "git.rev_parse(options.to + '^{commit}')" or just modify git.rev_parse
> to do it (and git_id to avoid it).

I prefer to work with software that understands what I mean and tells me that I cannot do it. It makes it easier to understand what is possible and how the command is working.

Recognizing patch names in some commands but not others would be annoying and inconsistent. Dumbing downs interactive software on purpose is probably not worth the trouble.

-- 
Regards,
Pavel Roskin
Catalin Marinas· Oct 3, 2007, 20:35 UTC · re: Pavel Roskin · lore

Re: [PATCH] Support tags in uncommit - use git_id instead of rev_parse

On 02/10/2007, Pavel Roskin <proski@gnu.org> wrote:
Show 12 quoted lines
> On Mon, 2007-10-01 at 23:00 +0100, Catalin Marinas wrote:
> > To allow tags, maybe just pass something like
> > "git.rev_parse(options.to + '^{commit}')" or just modify git.rev_parse
> > to do it (and git_id to avoid it).
>
> I prefer to work with software that understands what I mean and tells me
> that I cannot do it.  It makes it easier to understand what is possible
> and how the command is working.
>
> Recognizing patch names in some commands but not others would be
> annoying and inconsistent.  Dumbing downs interactive software on
> purpose is probably not worth the trouble.

Without this patch, the 'stg uncommit -t patch' fails with 'Unknown revision: patch'. With the patch applied, it still fails but with 'Commit ... does not have exactly one parent'. I don't say that the first one is good but I don't think the latter is clearer. The 'stg uncommit --help' states that the '--to' option takes a commit argument but if one passes a patch name the error message gets pretty confusing.

-- 
Catalin
Pavel Roskin· Oct 3, 2007, 21:44 UTC · re: Catalin Marinas · lore

Re: [PATCH] Support tags in uncommit - use git_id instead of rev_parse

On Wed, 2007-10-03 at 21:35 +0100, Catalin Marinas wrote:
Show 7 quoted lines
> Without this patch, the 'stg uncommit -t patch' fails with 'Unknown
> revision: patch'. With the patch applied, it still fails but with
> 'Commit ... does not have exactly one parent'. I don't say that the
> first one is good but I don't think the latter is clearer. The 'stg
> uncommit --help' states that the '--to' option takes a commit argument
> but if one passes a patch name the error message gets pretty
> confusing.

Actually, 'Commit ... does not have exactly one parent' means that stg misinterpreted the patch name as some non-existing hash and started iterating back until it hit the first merge.

Perhaps stgit should make sure that the hash is valid before walking the commit tree. If it's not, stgit could provide a better message.

-- 
Regards,
Pavel Roskin
Catalin Marinas· Oct 7, 2007, 21:06 UTC · re: Pavel Roskin · lore

Re: [PATCH] Support tags in uncommit - use git_id instead of rev_parse

On 03/10/2007, Pavel Roskin <proski@gnu.org> wrote:
Show 16 quoted lines
> On Wed, 2007-10-03 at 21:35 +0100, Catalin Marinas wrote:
>
> > Without this patch, the 'stg uncommit -t patch' fails with 'Unknown
> > revision: patch'. With the patch applied, it still fails but with
> > 'Commit ... does not have exactly one parent'. I don't say that the
> > first one is good but I don't think the latter is clearer. The 'stg
> > uncommit --help' states that the '--to' option takes a commit argument
> > but if one passes a patch name the error message gets pretty
> > confusing.
>
> Actually, 'Commit ... does not have exactly one parent' means that stg
> misinterpreted the patch name as some non-existing hash and started
> iterating back until it hit the first merge.
>
> Perhaps stgit should make sure that the hash is valid before walking the
> commit tree.  If it's not, stgit could provide a better message.

OK, I applied your patch but I'll have to look into the error message to make it more meaningful. Thanks.

-- 
Catalin

← back to recent threads