git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pierre Habouzit <madcoder@debian.org>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH 9/5] Migrate git-checkout.sh to use git-rev-parse --parseopt  --keep-dashdash
Date: Sun, 04 Nov 2007 15:18:09 +0100	[thread overview]
Message-ID: <20071104141809.GG26269@artemis.corp> (raw)
In-Reply-To: <Pine.LNX.4.64.0711041343470.4362@racer.site>

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

On Sun, Nov 04, 2007 at 01:48:36PM +0000, Johannes Schindelin wrote:
> > Pierre Habouzit <madcoder@debian.org> writes:
> > It takes on the standard input the specification of the options to parse
> > and understand, and echoes on the standard ouput a line suitable for 
> > `sh(1)` `eval` to replace the arguments with normalized ones.
> 
> Why not go the full nine yards and output something which when eval'ed 
> sets the variables correctly (taking the variable names from the option 
> names; long name if available, otherwise short one)?  It can also set the 
> command line arguments to what's left after option parsing, with a "set" 
> call.

We could do that, though it's not as great as it looks like at the first
glance. If you want -vvv to work like an accumulator, then you need a
really more complex approach in the C code. To enter the gory details,
git-rev-parse --parseopt uses a callback that deals with options and
their arguments one by one, then appends a delimiter to tell the shell
script that only arguments follow, and then appends the arguments the
option parser left alone.

It does not deal with the semantics that the C has available at all,
it's up to the shell script to decide which is better.

My goal with this is not really to do all the work for the shell script
author, but rather to the user: it's not because a porcelain is new (or
not a builtin yet) that it should have a creepy interface. If it helps
the programmer as a side effect, then it's great, but this series really
is about usability to me.

Of course we can do what you propose, but it will probably be quite
sophisticated and looks to me like an overkill to what shell builtins
really are used for: prototyping a new porcelain until it becomes a new
full blown C-builtin.  I do believe in simplicity after all :)

> And to prevent funny games with "PARSEOPT_OPTS=blabla git xyz", why not 
> provide a function in git-sh-setup which takes the string as argument, and 
> is called _after_ sourcing git-sh-setup?

This one is quite a non issue, it's only used for keep-dashdash, and I
see no other need in the near future. And if the need arise, it'll still
be doable any time. So for now I've taken a non-generic way to do that,
and I believe it's fine as it is.
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

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

  reply	other threads:[~2007-11-04 14:18 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-11-02 22:39 let's retry this based on git-rev-parse --parseopt Pierre Habouzit
2007-11-02 22:39 ` [PATCH 1/5] Add a parseopt mode to git-rev-parse to bring parse-options to shell scripts Pierre Habouzit
2007-11-02 22:39   ` [PATCH 2/5] Update git-sh-setup(1) to allow transparent use of git-rev-parse --parseopt Pierre Habouzit
2007-11-02 22:39     ` [PATCH 3/5] Migrate git-clean.sh to use " Pierre Habouzit
2007-11-02 22:39       ` [PATCH 4/5] Migrate git-clone " Pierre Habouzit
2007-11-02 22:39         ` [PATCH 5/5] Migrate git-am.sh " Pierre Habouzit
2007-11-03  9:55           ` Alex Riesen
2007-11-03 11:54             ` Pierre Habouzit
2007-11-03 12:05               ` Pierre Habouzit
2007-11-03 17:50         ` [PATCH 5/5 FIX SPACING] " Pierre Habouzit
2007-11-03 17:50           ` [PATCH 6/5] Migrate git-merge.sh " Pierre Habouzit
2007-11-03 17:50             ` [PATCH 8/5] Migrate git-instaweb.sh " Pierre Habouzit
2007-11-03 17:50               ` [PATCH 9/5] Migrate git-checkout.sh to use git-rev-parse --parseopt --keep-dashdash Pierre Habouzit
2007-11-03 17:50                 ` [PATCH 10/5] Migrate git-quiltimport.sh to use git-rev-parse --parseopt Pierre Habouzit
2007-11-03 18:35                   ` [PATCH 11/5] Migrate git-repack.sh " Pierre Habouzit
2007-11-04  7:44                 ` [PATCH 9/5] Migrate git-checkout.sh to use git-rev-parse --parseopt --keep-dashdash Junio C Hamano
2007-11-04  9:03                   ` Pierre Habouzit
2007-11-04 13:48                   ` Johannes Schindelin
2007-11-04 14:18                     ` Pierre Habouzit [this message]
2007-11-04  7:43     ` [PATCH 2/5] Update git-sh-setup(1) to allow transparent use of git-rev-parse --parseopt Junio C Hamano
2007-11-04  9:11       ` [UPDATED PATCH 3/5] " Pierre Habouzit
2007-11-04  9:11         ` [UPDATED PATCH 5/5] Migrate git-clone to use " Pierre Habouzit
2007-11-04  9:11           ` [UPDATED PATCH 9/5] Migrate git-checkout.sh to use git-rev-parse --parseopt --keep-dashdash Pierre Habouzit
2007-11-04  9:14       ` [PATCH 2/5] Update git-sh-setup(1) to allow transparent use of git-rev-parse --parseopt Pierre Habouzit
2007-11-04  9:15         ` Pierre Habouzit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20071104141809.GG26269@artemis.corp \
    --to=madcoder@debian.org \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).