libtool-patches
[Top][All Lists]
Advanced

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

Re: [PATCH 2/6] Fix partial commit support.


From: Ralf Wildenhues
Subject: Re: [PATCH 2/6] Fix partial commit support.
Date: Tue, 31 Aug 2010 19:46:59 +0200
User-agent: Mutt/1.5.20 (2010-04-22)

Hello Gary,

* Gary V. Vaughan wrote on Tue, Aug 31, 2010 at 08:43:16AM CEST:
> * clcommit.m4sh (func_commit): Commit only staged files instead
> of passing `-a' when no file list was given on the command line.

FWIW, I've never used clcommit since we moved to git; I just use
plain git.  Here's a couple of comments though:

> --- a/clcommit.m4sh
> +++ b/clcommit.m4sh
> @@ -268,11 +268,18 @@ func_commit ()
>  
>      sleep 1 # give the user some time for a ^C
>  
> -    subject=`git status 2>/dev/null | $SED -n 's/^#.*[mad][ode][dl].*ed: 
> *//p'`
> -    test $# -gt 0 && subject="$@"
> +    if test $# -gt 0; then
> +      # Given a list of files to commit from the command line: unstage all
> +      # files, and add back those in the list...
> +      for staged in `$GIT status -s --porcelain 2>/dev/null | $SED -n 's/^M. 
> //p'`
> +      do
> +        $GIT reset -- $staged
> +      done

I wouldn't use a commit script that messed with my working tree or index
in uncontrollable ways.  The 'git status -s' format is described in
detail in the git-status(1) man page, and your extraction is not safe in
the presence of arbitrary changes, for example renames, file deletions,
file additions, unmerged files.

More generally, I would consider this script dangerous in trying to hide
the complexity of git in possibly unexpected ways.

Come to think, why do we have this script in the Libtool tree at all?
There is no apparent connection to the Libtool functionality.  Maybe
this, as well as announce-gen, could live in their own project?

> +      $GIT add ${1+"$@"}

This ${1+"$@"} doesn't make sense here: either you are guaranteed here
to have at least one positional parameter, in which case you can just
use "$@", or you don't, then you have an error here because git add
requires arguments.

> +    fi
> +    # ...otherwise, by default, only staged files are committed.
>  
> -    test $# -gt 0 || { set dummy -a; shift; }
> -    func_show_eval "$GIT commit $git_flags -F $log_file address@hidden" 
> "exit $EXIT_FAILURE"
> +    func_show_eval "$GIT commit $git_flags -F $log_file" "exit $EXIT_FAILURE"
>  
>      $opt_push && {
>        func_show_eval "$GIT push"

Cheers,
Ralf



reply via email to

[Prev in Thread] Current Thread [Next in Thread]