Git development
 help / color / mirror / Atom feed
* Re: git-walkthrough-add script
From: Pedro Melo @ 2008-01-04 23:26 UTC (permalink / raw)
  To: William Morgan; +Cc: Git Mailing List
In-Reply-To: <1199426431-sup-6092@south>

Hi,

On Jan 4, 2008, at 6:14 AM, William Morgan wrote:

> I've written a little script to do darcs-style hunk-by-hunk
> walkthroughs. It's based on the git-hunk-commit script that was  
> floating
> around. Maybe someone else will find it useful.
>
> http://git-wt-commit.rubyforge.org/

Maybe I'm doing something wrong, but comparing git-add -p with your  
script, git-add -p is more darcs'ish. With git-add, if I have several  
changes in the same file, I get to choose per hunk.

You script asks me to add all the changes in the same file at once.

Best regards,
-- 
Pedro Melo
Blog: http://www.simplicidade.org/notes/
XMPP ID: melo@simplicidade.org
Use XMPP!

^ permalink raw reply

* Re: git-walkthrough-add script
From: Junio C Hamano @ 2008-01-04 23:28 UTC (permalink / raw)
  To: Pedro Melo; +Cc: William Morgan, Git Mailing List
In-Reply-To: <2CC98B8C-CBB1-4C26-8C94-B152A4D02DDC@simplicidade.org>

Pedro Melo <melo@simplicidade.org> writes:

> Maybe I'm doing something wrong, but comparing git-add -p with your
> script, git-add -p is more darcs'ish. With git-add, if I have several
> changes in the same file, I get to choose per hunk.

Heh, thanks from somebody who did "add -i" who admits he used
darcs for a few months ;-).

^ permalink raw reply

* Re: git-walkthrough-add script
From: William Morgan @ 2008-01-05  0:02 UTC (permalink / raw)
  To: Git Mailing List
In-Reply-To: <2CC98B8C-CBB1-4C26-8C94-B152A4D02DDC@simplicidade.org>

Excerpts from Pedro Melo's message of Fri Jan 04 15:26:41 -0800 2008:
> You script asks me to add all the changes in the same file at once.

It shouldn't. The whole point is to walk through and allow selection on
a per-hunk basis.

Maybe I'm parsing the output of git diff incorrectly and not picking up
hunk boundaries? Would you mind sending the output of a git diff that
results in this behavior?

-- 
William <wmorgan-git@masanjin.net>

^ permalink raw reply

* Re: git-walkthrough-add script
From: Pedro Melo @ 2008-01-05  0:17 UTC (permalink / raw)
  To: William Morgan; +Cc: Git Mailing List
In-Reply-To: <1199491057-sup-5588@south>

Hi,

On Jan 5, 2008, at 12:02 AM, William Morgan wrote:
> Excerpts from Pedro Melo's message of Fri Jan 04 15:26:41 -0800 2008:
>> You script asks me to add all the changes in the same file at once.
>
> It shouldn't. The whole point is to walk through and allow  
> selection on
> a per-hunk basis.
>
> Maybe I'm parsing the output of git diff incorrectly and not  
> picking up
> hunk boundaries? Would you mind sending the output of a git diff that
> results in this behavior?

I used a new git repo, just to test your script.

The diff is:

------ snip
diff --git a/a b/a
index b6b8cab..456bb2d 100644
--- a/a
+++ b/a
@@ -1,4 +1,5 @@
  asasas
+****
  assas
  assas
  asas
@@ -37,6 +38,7 @@ asasas
  assas
  assas
  asas
+=fsdfsdfsadfasdfsad=

  asasas
  assas
@@ -46,5 +48,6 @@ asas
  asasas
  assas
  assas
+asdasdasdasda=DASD=AS=DA=SD=
  asas

------ snip

Best regards
-- 
Pedro Melo
Blog: http://www.simplicidade.org/notes/
XMPP ID: melo@simplicidade.org
Use XMPP!

^ permalink raw reply related

* Re: [PATCH] add--interactive: allow diff colors without interactive colors
From: Junio C Hamano @ 2008-01-05  0:20 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20080104083521.GB3354@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Users with color.diff set to true/auto will not see color in
> "git add -i" unless they also set color.interactive.
>
> However, some users may want just one without the other, so
> there's no reason to tie them together.
>
> Note that there is now no way to have color on for "git
> diff" but not for diffs from "git add -i"; such a
> configuration seems unlikely, though.

Although I would agree with what this patch does, I think you
are contradicting with yourself in the above justification.
Some users may want to color "git diff" output but not
interaction with "git add -i", and that's also "just one without
the other", but you just tied them together, only differently,
and "seems unlikely" is a rather weak excuse.

The justification should instead be: having more independent
knobs is not necessarily better.  The user should not have to
tweak too many knobs.

In the longer term, I think we should try reducing the number of
knobs by giving "color.git" that allows you to pretend as if all
of the "color.interactive", "color.diff", "color.status",
"color.someothercolorizedcommand" are all set.  I do not think
being able to control the use of colors per command is giving
much other than confusion to the user.

That may not be so easy with the current structure of the config
reader, though.

^ permalink raw reply

* Re: [PATCH] git stash: one bug and one feature request
From: Junio C Hamano @ 2008-01-05  0:29 UTC (permalink / raw)
  To: Marco Costalba; +Cc: Git Mailing List
In-Reply-To: <e5bfff550801040814n82f34b2g17c485a207093440@mail.gmail.com>

"Marco Costalba" <mcostalba@gmail.com> writes:

> Otherwise git-stash is unusable by scripts that check
> stderr to detect fail/success of launched command.

Sorry, but I happen to disagree with your notion of "having
something on stderr is an error" to begin with.  I think scripts
written that way are either simply bogus, or are working around
a defect in the underlying command it calls (perhaps it does not
signal error with exit status properly).

A command that produces machine parsable output should write
that out to stdout, and if it needs to emit other informational
messages meant for human consumption (this includes progress
bars), that should be sent to stderr so that scripts can get the
meat of the output without having to filter cruft out.

If the command does not signal an error by exiting with non-zero
status, that would be a bug indeed and you can fix that instead,
I think.

^ permalink raw reply

* Re: [PATCH] git stash: one bug and one feature request
From: Junio C Hamano @ 2008-01-05  0:31 UTC (permalink / raw)
  To: Marco Costalba
  Cc: Brandon Casey, Pascal Obry, Junio C Hamano, Git Mailing List
In-Reply-To: <e5bfff550801041046p534b4869l2919494a8e4ef711@mail.gmail.com>

"Marco Costalba" <mcostalba@gmail.com> writes:

> I'm very bad at naming, so I don't argue any more.
>
> Just one thing (that is the only that matters) call this command as
> you want but let it take one argument, the name of the reflog to
> remove:
>
> git stash drop stash@{3}
>
> should be allowed.
>
> git stash drop
>
> defaults to  stash@{0}

I do not care the wording either way, but my prediction is that
people will mistype "stash clear" when they meant "stash drop",
and we will end up not allowing the implicit "drop the top one
only" behaviour.

^ permalink raw reply

* Re: Trouble importing from public CVS repo
From: Jeff Garzik @ 2008-01-05  0:40 UTC (permalink / raw)
  To: Sean; +Cc: Git Mailing List, Matthias Urlichs
In-Reply-To: <BAYC1-PASMTP15EA2E9A1145CD66AF7C4DAE4C0@CEZ.ICE>

Sean wrote:
> diff --git a/git-cvsimport.perl b/git-cvsimport.perl
> index 6d8ff93..357665d 100755
> --- a/git-cvsimport.perl
> +++ b/git-cvsimport.perl
> @@ -421,7 +421,7 @@ sub _line {
>  			$res += $self->_fetchfile($fh, $cnt);
>  		} else {
>  			chomp $line;
> -			if ($line eq "ok") {
> +			if ($line eq "ok" or $line =~ /^error/i) {
>  				# print STDERR "S: ok (".length($res).")\n";
>  				return $res;
>  			} elsif ($line =~ s/^E //) {

Kick ass, this local hack worked quite well.  Thanks :)

	Jeff

^ permalink raw reply

* Re: git-walkthrough-add script
From: Miklos Vajna @ 2008-01-05  1:00 UTC (permalink / raw)
  To: Pedro Melo; +Cc: William Morgan, Git Mailing List
In-Reply-To: <2CC98B8C-CBB1-4C26-8C94-B152A4D02DDC@simplicidade.org>

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

On Fri, Jan 04, 2008 at 11:26:41PM +0000, Pedro Melo <melo@simplicidade.org> wrote:
> Maybe I'm doing something wrong, but comparing git-add -p with your script, 
> git-add -p is more darcs'ish. With git-add, if I have several changes in 
> the same file, I get to choose per hunk.

hm, if you want a darcs-like record interface.. darcs record asks you if
you want to include a newly added file in a commit or not,
git-walkthrough-add won't do so :s

you might want to have a look at dg record
(http://git.frugalware.org/repos/pacman-tools/darcs-git.py) which does
this for you.

- VMiklos

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

^ permalink raw reply

* [PATCH] builtin-reflog.c: typo ref --> argv[i], could cause segfault
From: Brandon Casey @ 2008-01-05  1:11 UTC (permalink / raw)
  To: Git Mailing List, Junio C Hamano

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 builtin-reflog.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-reflog.c b/builtin-reflog.c
index f422693..5e54989 100644
--- a/builtin-reflog.c
+++ b/builtin-reflog.c
@@ -394,7 +394,7 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)
 		int recno;
 
 		if (!spec) {
-			status |= error("Not a reflog: %s", ref);
+			status |= error("Not a reflog: %s", argv[i]);
 			continue;
 		}
 
-- 
1.5.4.rc2.1119.g6fdf-dirty

^ permalink raw reply related

* Re: [EGIT PATCH] Remove directory entries from Structure Compare view.
From: Robin Rosenberg @ 2008-01-05  1:11 UTC (permalink / raw)
  To: Roger C. Soares; +Cc: git, Shawn O. Pearce
In-Reply-To: <200801040011.17934.rogersoares@intelinet.com.br>

fredag 04 januari 2008 skrev Roger C. Soares:
> Date: Wed, 2 Jan 2008 22:51:33 -0200
> 
> Signed-off-by: Roger C. Soares <rogersoares@intelinet.com.br>
> ---
> Please evaluate.

Very much I like it. I will apply it. My guess is some people want
to make this an option in the compare viewer, but we can add that later.

-- robin

^ permalink raw reply

* Re: [PATCH] builtin-reflog.c: typo ref --> argv[i], could cause segfault
From: Junio C Hamano @ 2008-01-05  1:22 UTC (permalink / raw)
  To: Brandon Casey; +Cc: Git Mailing List
In-Reply-To: <477ED949.1030909@nrlssc.navy.mil>

Brandon Casey <casey@nrlssc.navy.mil> writes:

> Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
> ---
>  builtin-reflog.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/builtin-reflog.c b/builtin-reflog.c
> index f422693..5e54989 100644
> --- a/builtin-reflog.c
> +++ b/builtin-reflog.c
> @@ -394,7 +394,7 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)
>  		int recno;
>  
>  		if (!spec) {
> -			status |= error("Not a reflog: %s", ref);
> +			status |= error("Not a reflog: %s", argv[i]);
>  			continue;
>  		}

Bad Brandon!  Looking at 'next' while everybody is asked to work
on finding and fixing bugs on 'master' while -rc freeze.

Just kidding.  Thanks, the code is obviously correct, and will
queue.

I am not so surprised to see bugs in relatively new code that is
only in 'next', especially this one that no Porcelains already
use.

^ permalink raw reply

* [PATCH] git-stash: add new 'drop' subcommand
From: Brandon Casey @ 2008-01-05  1:31 UTC (permalink / raw)
  To: Git Mailing List, Junio C Hamano
In-Reply-To: <1199495198-26270-1-git-send-email-casey@nrlssc.navy.mil>

This allows a single stash entry to be deleted. It takes an
optional argument which is a stash reflog entry. If no
arguments are supplied, stash@{0} is used.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---


Thus far I haven't been a big user of git stash, but I plan to
use it more and I expect to use 'drop' more often than
'clear'. I expect in the common case there will be a single
stash, and 'drop' will be sufficient. For the case where there
are many stashes and I want to remove one, 'drop' is required.
'git stash clear' will become a command that I give special
attention to just like 'rm -f *'.

I'm not sure if there is a proper way to get 'stash@{0}' from
'refs/stash' so I kept my usage of that former string outside
of the drop_stash() function.

Comments welcome, especially if there is a more appropriate
way to do this.

-brandon


 Documentation/git-stash.txt |    7 ++++++-
 git-stash.sh                |   29 ++++++++++++++++++++++++++++-
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index c0147b9..b89eadb 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -8,7 +8,7 @@ git-stash - Stash the changes in a dirty working directory away
 SYNOPSIS
 --------
 [verse]
-'git-stash' (list | show [<stash>] | apply [<stash>] | clear)
+'git-stash' (list | show [<stash>] | apply [<stash>] | clear | drop [<stash>])
 'git-stash' [save] [message...]
 
 DESCRIPTION
@@ -81,6 +81,11 @@ clear::
 	Remove all the stashed states. Note that those states will then
 	be subject to pruning, and may be difficult or impossible to recover.
 
+drop [<stash>]::
+
+	Remove a single stashed state from the stash list. When no `<stash>`
+	is given, it removes the latest one. i.e. `stash@\{0}`
+
 
 DISCUSSION
 ----------
diff --git a/git-stash.sh b/git-stash.sh
index 06cb177..a789a53 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -1,7 +1,7 @@
 #!/bin/sh
 # Copyright (c) 2007, Nanako Shiraishi
 
-USAGE='[  | save | list | show | apply | clear | create ]'
+USAGE='[  | save | list | show | apply | clear | create | drop ]'
 
 SUBDIRECTORY_OK=Yes
 OPTIONS_SPEC=
@@ -192,6 +192,24 @@ apply_stash () {
 	fi
 }
 
+drop_stash () {
+	if ! have_stash
+	then
+		echo >&2 'No stash entries to drop'
+		exit 0
+	fi
+
+	# Verify supplied argument looks like a stash entry
+	s=$(git rev-parse --revs-only --no-flags --default $ref_stash "$@") &&
+	git rev-parse --verify "$s:"   > /dev/null 2>&1 &&
+	git rev-parse --verify "$s^1:" > /dev/null 2>&1 &&
+	git rev-parse --verify "$s^2:" > /dev/null 2>&1 ||
+		die "$*: not a valid stashed state"
+
+	git reflog delete "$@" && echo "Dropped $@ ($s)" ||
+		die "$*: Could not drop stash entry"
+}
+
 # Main command set
 case "$1" in
 list)
@@ -225,6 +243,15 @@ create)
 	fi
 	create_stash "$*" && echo "$w_commit"
 	;;
+drop)
+	shift
+	if test $# = 0
+	then
+		set -- "stash@{0}"
+	fi
+	drop_stash "$@" &&
+	(git rev-parse --verify "stash@{0}" > /dev/null 2>&1 || clear_stash)
+	;;
 *)
 	if test $# -eq 0
 	then
-- 
1.5.4.rc2.1119.g6fdf-dirty

^ permalink raw reply related

* Re: [PATCH] builtin-reflog.c: typo ref --> argv[i], could cause segfault
From: Brandon Casey @ 2008-01-05  1:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vzlvlf4n9.fsf@gitster.siamese.dyndns.org>

Junio C Hamano wrote:
> Bad Brandon!  Looking at 'next' while everybody is asked to work
> on finding and fixing bugs on 'master' while -rc freeze.

No more, I promise. :)

-brandon

^ permalink raw reply

* Re: [PATCH] git-stash: add new 'drop' subcommand
From: Junio C Hamano @ 2008-01-05  1:47 UTC (permalink / raw)
  To: Brandon Casey; +Cc: Git Mailing List
In-Reply-To: <477EDDD4.5060509@nrlssc.navy.mil>

Brandon Casey <casey@nrlssc.navy.mil> writes:

> I'm not sure if there is a proper way to get 'stash@{0}' from
> 'refs/stash' so I kept my usage of that former string outside
> of the drop_stash() function.

Doesn't "$refs_stash@{0}" (which would give refs/stash@{0} not
stash@{0}) work for you?

> diff --git a/git-stash.sh b/git-stash.sh
> index 06cb177..a789a53 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -1,7 +1,7 @@
>  #!/bin/sh
>  # Copyright (c) 2007, Nanako Shiraishi
>  
> -USAGE='[  | save | list | show | apply | clear | create ]'
> +USAGE='[  | save | list | show | apply | clear | create | drop ]'

Might want to put drop next to clear, but that is minor.

> +drop_stash () {
> +	if ! have_stash
> +	then
> +		echo >&2 'No stash entries to drop'
> +		exit 0
> +	fi
> +
> +	# Verify supplied argument looks like a stash entry
> +	s=$(git rev-parse --revs-only --no-flags --default $ref_stash "$@") &&
> +	git rev-parse --verify "$s:"   > /dev/null 2>&1 &&
> +	git rev-parse --verify "$s^1:" > /dev/null 2>&1 &&
> +	git rev-parse --verify "$s^2:" > /dev/null 2>&1 ||
> +		die "$*: not a valid stashed state"
> +
> +	git reflog delete "$@" && echo "Dropped $@ ($s)" ||

The second $@ is inconsistent with the next line's use of $*; intentional?

> +		die "$*: Could not drop stash entry"
> +}

> +drop)
> +	shift
> +	if test $# = 0
> +	then
> +		set -- "stash@{0}"
> +	fi
> +	drop_stash "$@" &&
> +	(git rev-parse --verify "stash@{0}" > /dev/null 2>&1 || clear_stash)

Curious.

 (1) Why not do the clearing inside drop_stash?

 (2) Why is clearning necessary in the first place (iow,
     shouldn't "reflog delete" take care of that)?

Other than that, nicely done.

^ permalink raw reply

* Re: [PATCH] git-filter-branch could be confused by similar names
From: Junio C Hamano @ 2008-01-05  1:17 UTC (permalink / raw)
  To: Dmitry Potapov; +Cc: git, Johannes Schindelin
In-Reply-To: <20080104155114.GS3373@dpotapov.dyndns.org>

Dmitry Potapov <dpotapov@gmail.com> writes:

> On Thu, Jan 03, 2008 at 01:27:27PM -0800, Junio C Hamano wrote:
>> ... I had an
>> impression that we try to stick to a subset of BRE (namely, no
>> \{m,n\}, [::], [==], nor [..]).
>
> I was not aware about this policy, and I am not aware about
> existing any grep that does not grok the expressions I used
> above. So, I thought they are commonly accepted, but I might
> be wrong.

Well I might be wrong too, as I did not vet all the existing use
of grep in our code.  That's why I said I had "an impression".

Now I have ("git grep 'grep ' -- 'git-*.sh'"), and it seems to
be that we do stick to a narrow subset of BRE.

 * We do not use \{m,n\};

 * We do not use -E;

 * We do not use ? nor + (which are \{0,1\} and \{1,\}
   respectively in BRE) but that goes without saying as these
   are ERE elements not BRE (note that \? and \+ you wrote are
   not even part of BRE -- making them accessible from BRE is a
   GNU extension).

IOW, our scripts' use of grep is very 80'sh ;-)

I do not mind using things that are available in POSIX BRE, but
let's not rely on GNU extension that may cause issues to other
people.

Other things I noticed while looking at "t/*.sh":

 * t/t3600-rm.sh and t/5401-update-hooks.sh have unnecessary
   uses of egrep that can instead be grep;

 * t/t3800-mktag.sh uses egrep but I think it can be grep; also,
   I think the use of temporary file expect.pat is unnecessary.

 * t/t5510-fetch.sh does use '^[0-9a-f]\{40\} '.

 * t/t7001-mv.sh uses -E only to use '.+' when it can just as
   easily say '..*' instead.

 * t/t7600-merge.sh has two occurrences of (GNU extended) " \+"
   that should be "  *" for portability.  An alternative is to
   use grep -E and say " +" instead, but then the other plus
   sign on the same line needs to be quoted.

Other than the one in 5510 I consider them log hanging fruits
for janitors.

^ permalink raw reply

* Re: git-walkthrough-add script
From: William Morgan @ 2008-01-05  2:37 UTC (permalink / raw)
  To: Git Mailing List
In-Reply-To: <20080104210751.GB26248@coredump.intra.peff.net>

Excerpts from Jeff King's message of Fri Jan 04 13:07:51 -0800 2008:
> But if you have interface improvement suggestions for "git-add -i" or
> "git-add -p", I'm sure they would be well-received (post-1.5.4
> release, of course).

Not to show off my git newbness any more than necessary, but when I run
both git-add -i and git-add -p, I see exactly the same output and
interface. Are they meant to be different? This is with both master and
next branches of the git repo.

  ~/devel/sup$ ~/devel/git/installed/bin/git-add -i
             staged     unstaged path
    1:    unchanged        +1/-0 doc/TODO
    2:    unchanged        +1/-1 lib/sup/message.rb
    3:    unchanged      +28/-43 lib/sup/thread.rb
  
  *** Commands ***
    1: status       2: update       3: revert       4: add untracked
    5: patch        6: diff         7: quit         8: help
  What now> 
  Bye.
  ~/devel/sup$ ~/devel/git/installed/bin/git-add -p
             staged     unstaged path
    1:    unchanged        +1/-0 doc/TODO
    2:    unchanged        +1/-1 lib/sup/message.rb
    3:    unchanged      +28/-43 lib/sup/thread.rb
  
  *** Commands ***
    1: status       2: update       3: revert       4: add untracked
    5: patch        6: diff         7: quit         8: help
  What now> 
  Bye.

-- 
William <wmorgan-git@masanjin.net>

^ permalink raw reply

* Re: git-walkthrough-add script
From: Junio C Hamano @ 2008-01-05  2:50 UTC (permalink / raw)
  To: William Morgan; +Cc: Git Mailing List
In-Reply-To: <1199500441-sup-4067@south>

William Morgan <wmorgan-git@masanjin.net> writes:

> Excerpts from Jeff King's message of Fri Jan 04 13:07:51 -0800 2008:
>> But if you have interface improvement suggestions for "git-add -i" or
>> "git-add -p", I'm sure they would be well-received (post-1.5.4
>> release, of course).
>
> Not to show off my git newbness any more than necessary, but when I run
> both git-add -i and git-add -p, I see exactly the same output and
> interface. Are they meant to be different?

They are meant to be different.

    $ git reset --hard
    $ echo >>Makefile
    $ echo >>psql/Makefile
    $ git add -p
    diff --git a/Makefile b/Makefile
    index a2177bc..eb250b0 100644
    --- a/Makefile
    +++ b/Makefile
    @@ -54,3 +54,4 @@ snapdiff ::
            latest=`ls -1dr $(_snap)/release-????-??-?? | head -n 1` &&
            \
            diff -X dontdiff -ru "$$latest" .

    +
    Stage this hunk [y/n/a/d/?]? ^C

    $ git add -i
               staged     unstaged path
      1:    unchanged        +1/-0 Makefile
      2:    unchanged        +1/-0 psql/Makefile

    *** Commands ***
      1: [s]tatus     2: [u]pdate     3: [r]evert     4: [a]dd
      untracked
      5: [p]atch      6: [d]iff       7: [q]uit       8: [h]elp
    What now> ^C

There may be something broken with your git installation.

^ permalink raw reply

* Re: git-walkthrough-add script
From: Junio C Hamano @ 2008-01-05  2:55 UTC (permalink / raw)
  To: William Morgan; +Cc: Git Mailing List
In-Reply-To: <7vprwhf0kf.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> William Morgan <wmorgan-git@masanjin.net> writes:
>
>> Not to show off my git newbness any more than necessary, but when I run
>> both git-add -i and git-add -p, I see exactly the same output and
>> interface. Are they meant to be different?
>
> They are meant to be different.
> ...
> There may be something broken with your git installation.

A shot in the dark.  Do you have more than one installation of
git, one of which is ancient and whose git-add--interactive does
not even have --patch option?

^ permalink raw reply

* Re: git-walkthrough-add script
From: William Morgan @ 2008-01-05  2:54 UTC (permalink / raw)
  To: Git Mailing List
In-Reply-To: <20080105010011.GV29972@genesis.frugalware.org>

Excerpts from Miklos Vajna's message of Fri Jan 04 17:00:11 -0800 2008:
> you might want to have a look at dg record
> (http://git.frugalware.org/repos/pacman-tools/darcs-git.py) which does
> this for you.

That's very similar. But I wasn't trying to mimick the darcs interface
exactly, just steal the parts I liked. For example, I don't actually
like being forced to commit immediately after staging some changes;
often I like to git diff --cached first.

Also, mine has color support. :)

-- 
William <wmorgan-git@masanjin.net>

^ permalink raw reply

* Re: git-walkthrough-add script
From: William Morgan @ 2008-01-05  3:02 UTC (permalink / raw)
  To: Git Mailing List
In-Reply-To: <7vlk75f0b6.fsf@gitster.siamese.dyndns.org>

Excerpts from Junio C Hamano's message of Fri Jan 04 18:55:41 -0800 2008:
> A shot in the dark.  Do you have more than one installation of git,
> one of which is ancient and whose git-add--interactive does not even
> have --patch option?

Precisely. Now I finally see what you were talking about.

Well, yup, this is pretty much what I had in mind. If only I had known.
Guess I'll throw away my script now!

-- 
William <wmorgan-git@masanjin.net>

^ permalink raw reply

* Re: [PATCH] add--interactive: allow diff colors without interactive colors
From: Jeff King @ 2008-01-05  3:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v3atdi0na.fsf@gitster.siamese.dyndns.org>

On Fri, Jan 04, 2008 at 04:20:09PM -0800, Junio C Hamano wrote:

> Although I would agree with what this patch does, I think you
> are contradicting with yourself in the above justification.
> Some users may want to color "git diff" output but not
> interaction with "git add -i", and that's also "just one without
> the other", but you just tied them together, only differently,
> and "seems unlikely" is a rather weak excuse.

Then let me state it another way that I think makes a better argument.
You have two knobs, color.interactive and color.diff, controlling
menu-coloring of git-add--interactive and controlling colorization of
git-diff output. Now we have a third thing that may be colorized: diff
output of git-add--interactive.

Let's assume that we don't want to add another color.interactive-diff
knob (though that is an option). That means that we have to tie the
colorization either to color.interactive or to color.diff. Right now we
subdivide it by command, so that the coloring of interactive diffs is
tied to color.interactive[1]. What I am proposing is to divide it by
_functionality_, so that by saying color.diff you mean "I like color
diffs, no matter where they are." And by saying color.interactive, you
mean "I like color interactive menus, no matter where they are." I think
it is much more likely that users will find that division useful. And
it's something we already do, since color.diff is respected not just by
git-diff, but by diffs produced by all programs, including the git-log
family.

[1]: Actually, we currently tie interactive diff coloring to "diff &&
interactive" which is even less useful. If I turn on color.interactive,
I still don't get colored diffs. But if I turn on color.diff, then
git-diff starts producing colored diffs. So you really can't represent
all choices, and I think the subdivision I outlined makes more sense (at
least it does to me).

> The justification should instead be: having more independent
> knobs is not necessarily better.  The user should not have to
> tweak too many knobs.
>
> In the longer term, I think we should try reducing the number of
> knobs by giving "color.git" that allows you to pretend as if all
> of the "color.interactive", "color.diff", "color.status",
> "color.someothercolorizedcommand" are all set.  I do not think
> being able to control the use of colors per command is giving
> much other than confusion to the user.

I'm not sure I agree with that; my problem here is that I _want_ to turn
a knob, but the functionality is tied to another knob. IOW, reducing the
number of knobs is going to make it worse.

That being said, all of those knobs _are_ confusing. In my case, I like
color. I just don't like the colors that color.interactive provides, so
I don't want to use them.  However, you can tune that quite a bit by
changing color.interactive.* (and just choosing "plain" for things you
don't want marked). Of course that still doesn't allow you to have
_different_ color settings between the diffs of git-diff and those of
git-add--interactive. But then, my point is that I don't think sane
users want that. They either want diffs colored or they don't, no matter
what command is producing them.

> That may not be so easy with the current structure of the config
> reader, though.

I don't think it's hard; the client code for the colors checks a
"color_foo" knob. It would just check "color_foo || color_all".

-Peff

^ permalink raw reply

* Re: git-walkthrough-add script
From: Jeff King @ 2008-01-05  3:43 UTC (permalink / raw)
  To: William Morgan; +Cc: Git Mailing List
In-Reply-To: <1199500828-sup-502@south>

On Fri, Jan 04, 2008 at 06:54:02PM -0800, William Morgan wrote:

> Also, mine has color support. :)

On the current master, try:

  git config color.interactive auto
  git config color.diff auto
  git add -i

-Peff

^ permalink raw reply

* Re: [PATCH] git-stash: add new 'drop' subcommand
From: Jeff King @ 2008-01-05  3:51 UTC (permalink / raw)
  To: Brandon Casey; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <477EDDD4.5060509@nrlssc.navy.mil>

On Fri, Jan 04, 2008 at 07:31:00PM -0600, Brandon Casey wrote:

> Thus far I haven't been a big user of git stash, but I plan to
> use it more and I expect to use 'drop' more often than
> 'clear'. I expect in the common case there will be a single

There was some discussion of a sensible name, but I don't recall seeing
a resolution on this: why not "clear stash@{0}" to clear one, and
"clear" to clear all? Otherwise, I foresee "git stash clear stash@{0}"
followed by "oops, I just deleted all of my stashes."

I guess you get "git stash drop" as a synonym for "git stash drop
stash@{0}" this way, but it just seems mean to users to make them
remember which of "drop" and "clear" does what they want.

-Peff

^ permalink raw reply

* Re: git-walkthrough-add script
From: Pedro Melo @ 2008-01-05  4:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: William Morgan, Git Mailing List
In-Reply-To: <7vprwhf0kf.fsf@gitster.siamese.dyndns.org>


On Jan 5, 2008, at 2:50 AM, Junio C Hamano wrote:
> They are meant to be different.
>
>     $ git reset --hard
>     $ echo >>Makefile
>     $ echo >>psql/Makefile
>     $ git add -p
>     diff --git a/Makefile b/Makefile
>     index a2177bc..eb250b0 100644
>     --- a/Makefile
>     +++ b/Makefile
>     @@ -54,3 +54,4 @@ snapdiff ::
>             latest=`ls -1dr $(_snap)/release-????-??-?? | head -n  
> 1` &&
>             \
>             diff -X dontdiff -ru "$$latest" .
>
>     +
>     Stage this hunk [y/n/a/d/?]? ^C

BTW, I'm using 1.5.4rc2 and this prompt shows:

Stage this hunk [y/n/a/d/j/J/?]?

but the help (after you press ?) also mentions:

k - leave this hunk undecided, see previous undecided hunk
K - leave this hunk undecided, see previous hunk
s - split the current hunk into smaller hunks

but those three options don't seem to work.

Best regards,
-- 
Pedro Melo
Blog: http://www.simplicidade.org/notes/
XMPP ID: melo@simplicidade.org
Use XMPP!

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox