Re: What should "git branch --merged master" do?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jul 08, 2008 at 06:49:13AM +0000, Junio C Hamano wrote:
> e8b404c (git-branch: add support for --merged and --no-merged, 2008-04-17)
> introduced "git branch --merged" to show the branches that are contained
> within the current HEAD.  I.e. the ones that you can say "git branch -d"
> and do not have to say "-D" to delete.
> 
> Currently "git branch --merged master" fails with a message:
> 
> 	fatal: A branch named 'master' already exists.
> 
> but --merged and its opposite --no-merged are in spirit very similar to
> another option --contains in that it is meant to be used as a way to
> influence which branches are _reported_, and not about creating a new
> branch.
> 
> Perhaps we should change them to default to HEAD (to retain the current
> behaviour) but take a commit, and show branches that are merged to the
> commit or not yet fully merged to the commit, respectively?
> 
> Incidentally, "git branch --with" fails without the mandatory commit
> argument, and if we are going to do the above, we probably should default
> the argument to HEAD as well.
> 
> Here is an attempt to update --with but I am not happy with it.
> 
> The patch makes
> 
> 	$ git branch --contains
> 
> work as expected, but breaks
> 
> 	$ git branch --contains master
> 
> You need to spell "git branch --contains=master" instead.  Which means it
> is a regression and cannot be applied.  I suspect I am not using
> parse_options() in an optimal way, but I did not find a way for my
> callback to tell "I consumed the next parameter and it was mine" or "The
> next one is not my optional parameter" back to the parse_options(), so
> probably parse_options() need to be fixed to update this without
> regression, I suspect.

  I actually see three ways.

  (1) quick and dirty: if we see at argv[argc - 1]
      --contains/--merge/--no-merge/... we do:

      argv[argc] = "HEAD" and use parse-options with a mandatory
      argument for --merge/--contains/... Okay this leaves the issue of
      the fact that we must NULL-terminate argv, but that's trivial
      (even if quite despisable).

  (2) big update of parse-options: we refactor callbacks so that they
      can decide if they take their argument or not. This removes the
      need for --long-opt=foo for options with optional arguments, but
      the refactor will be heavy. That's more or less what you suggest
      in your last § but I don't like it for many reasons so I wont
      enter the details.

  (3) inspired from (1) and (2), have a flag for options that says "I do
      take an argument, but if I'm the last option on the command line,
      please fake this argument for me.

  I really like (3) more FWIW as it doesn't generate ambiguous parsers
like (2) would, and it's not horrible like (1). And cherry on top it's
pretty trivial to implement I think.
-- 
·O·  Pierre Habouzit
··O                                                madcoder@xxxxxxxxxx
OOO                                                http://www.madism.org

Attachment: pgpnufmveuAPb.pgp
Description: PGP signature


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux