Git development
 help / color / mirror / Atom feed
* What's in git.git (stable frozen)
From: Junio C Hamano @ 2008-01-05 10:46 UTC (permalink / raw)
  To: git; +Cc: Tsugikazu Shibata, Marco Costalba, Jeff King, Dan McGee,
	Dmitry Potapov
In-Reply-To: <7vfxxtu5ov.fsf@gitster.siamese.dyndns.org>

We are not at -rc3 yet, but we will be soon.  What we have
accumulated in 'master' are mostly fixes, and the official
git-gui 0.9.1 is also included tonight.

I have to apologize that tonight I got a bit carried away
enjoying arguing for the sake of arguing.  Some patches that
might be worthy even though they are late in the cycle are not
in tonight's 'master', mostly because I have to sleep on them,
and partly because I am running out of time tonight.

 * Tsugikazu Shibata's git-diff hunk header change.

   I have a counterproposal that I think is more in line with
   the other parts of the system.  As the kernel project has
   ja_JP, ko_KR and zh_CN directories under Documentation these
   days, the issue this patch addresses is already real, and we
   would want to have a solution in 1.5.4, even though the topic
   was raised too late in the cycle.  I think my first two
   patches could be a good starting point for that.  I'd exclude
   the last patch in the series that acts on gitattributes for
   now.

 * Marco's git-stash changes to output to stdout.

   I'd probably apply this, with a slightly toned down commit
   log message.  Marco says some practice is standard, I
   disagreed, but that is not a reason to say "this practice is
   nonstandard and bad".  Simply saying "some do this and it is
   better to be helpful to them because there is no strong
   reason not to" would be good enough.

 * Jeff's git-add--interactive change to always honor color.diff
   regardless of color.interactive.

   I'd probably apply this, along with the patch to redefine
   what color.interactive means.  "git am -i" could also learn
   to use colors in the future.

   Incidentally I noticed that many of the color.diff.* palette
   options are read by "git-add -i" but never used by the
   script.  We might want to fix this while we are at it.

 * Dan McGee's workaround to breakage caused by changes in
   AsciiDoc 8.2.3.

   I have to do my usual "before-and-after comparison" with
   copies of AsciiDoc versions that should not be affected by
   the breakage, which I did not have time to do so far.  But
   this is probably a must-have before the release.

 * My patch to error out "git stash clear foobar".

   This should be applied; it is a good safety measure
   regardless of where that "git stash drop" thing would go.

An issue worth addressing before the release is still in limbo.

 * Dmitry's git-filter-branch fix to disambiguate the refs being
   rewritten.

   Addition of "git-rev-parse --symbolic-full" may solve this
   more cleanly than the patches in the discussion, but we
   haven't reached the conclusion of this thread yet.

Anything I missed?

----------------------------------------------------------------

* The 'master' branch has these since the last announcement.

Alex Riesen (1):
  Allow selection of different cleanup modes for commit messages

Arjen Laarhoven (1):
  Fix "git log --diff-filter" bug

Bernt Hansen (1):
  git-gui: Make commit log messages end with a newline

Eric Wong (2):
  git-svn: allow dcommit --no-rebase to commit multiple, dependent changes
  git-svn: unlink index files that were globbed, too

Grégoire Barbier (1):
  Fix double-free() in http-push.c:remote_exists()

Gustaf Hendeby (2):
  shortlog manpage documentation: work around asciidoc markup issues
  Documentation/user-manual.txt: fix typo

J. Bruce Fields (1):
  Documentation: fix remote.<name>.skipDefaultUpdate description

Jeff King (6):
  cvsimport: die on cvsps errors
  config: handle lack of newline at end of file better
  git-reset: refuse to do hard reset in a bare repository
  add a "basic" diff config callback
  diff: load funcname patterns in "basic" config
  diff: remove lazy config loading

Jim Meyering (2):
  Fix grammar nits in documentation and in code comments.
  Don't access line[-1] for a zero-length "line" from fgets.

Johannes Schindelin (1):
  Optimize prefixcmp()

Johannes Sixt (1):
  git-gui: Move frequently used commands to the top of the context menu.

Junio C Hamano (20):
  t7005: do not exit inside test.
  builtin-commit: fix amending of the initial commit
  builtin-commit: avoid double-negation in the code.
  Fix documentation of --first-parent in git-log and copy it to
    git-rev-list
  combine-diff: Fix path quoting
  Fix rewrite_diff() name quoting.
  contrib: resurrect scripted git-revert.
  GIT 1.5.4-rc2
  Documentation/git-submodule.txt: typofix
  "git pull --tags": error out with a better message.
  git-rebase -i behaves better on commits with incomplete messages
  git-rebase -i: clean-up error check codepath.
  lock_any_ref_for_update(): reject wildcard return from check_ref_format
  Update callers of check_ref_format()
  Uninline prefixcmp()
  git-clean: make "Would remove ..." path relative to cwd again
  t/t7600: avoid GNUism in grep
  t/t{3600,3800,5401}: do not use egrep when grep would do
  t/t3800: do not use a temporary file to hold expected result.
  Update draft release notes for 1.5.4

Marco Costalba (1):
  Document git-reset defaults to HEAD if no commit is given

Mark Levedahl (1):
  git-gui: Unconditionally use absolute paths with Cygwin

Martin Koegler (2):
  receive-pack: check object type of sha1 before using them as commits
  receive-pack: reject invalid refnames

Michael Stefaniuc (1):
  git-am: Run git gc only once and not for every patch.

Miklos Vajna (2):
  git-sh-setup: document git_editor() and get_author_ident_from_commit()
  t/t7001: avoid unnecessary ERE when using grep

Peter Karlsson (1):
  Added Swedish translation.

René Scharfe (1):
  Make "--pretty=format" parser a bit more careful.

Shawn O. Pearce (2):
  git-gui: Handle file mode changes (644->755) in diff viewer
  Improve error messages when int/long cannot be parsed from config

^ permalink raw reply

* Re: [PATCH] git-stash clear: refuse to work with extra parameter for now
From: Jeff King @ 2008-01-05 10:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: JM Ibanez, Brandon Casey, Git Mailing List
In-Reply-To: <7vy7b4bon9.fsf_-_@gitster.siamese.dyndns.org>

On Sat, Jan 05, 2008 at 01:35:54AM -0800, Junio C Hamano wrote:

> I think something along this line may be necessary to
> futureproof our users. 

I think it not only futureproofs, but it helps those who misunderstand
(or don't read) the documentation from accidentally losing work.

Acked-by: Jeff King <peff@peff.net>

-Peff

^ permalink raw reply

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

On Sat, Jan 05, 2008 at 01:28:25AM -0800, Junio C Hamano wrote:

> Who said anything about "interactive is limited to interactive
> menus" anywhere?  That is where we differ and what you do not
> seem to be getting.  I am talking about color.interactive that
> controls the whole user experience of interacting with "add -i".

I did, just now. I am "getting" it just fine, but am trying to propose
an equally valid, and IMHO more useful, semantic for
"color.interactive." (which btw, is a horrible name if you truly mean
"the behavior of git add -i" since there are _other_ interactive
commands. Why doesn't it turn on color for git-rebase -i?).

> What I am aiming at in longer term is to simplify things this way:
> 
>  * Users are categorized broadly into two groups.  The ones who
>    like colours and the ones who don't want colours at all.
>    color.git would control this (with backward compatibility
>    options per command such as color.diff and
>    color.interactive);

OK, that makes sense. But I disagree somewhat with the phrase "backward
compatibility."

>  * Minorities who want to disable colours for particular parts
>    of the UI have enough knobs to tweak in the form of palettes.

They do, but those knobs are a pain to use. Why are we making it so hard
for them when we _already_ have the nice knobs for them to use. IOW, for
these users, git after this change will be _more_ annoying to use. Not
to mention that we must keep those other knobs around for historical
reasons for some amount of time.

Why not just add the higher-level knob and leave the existing ones
(using the priority scheme I mentioned in my last mail)?

>    By definition this needs to address "particular parts", so
>    "color.$command.$context" variables (e.g. color.diff.new,
>    color.interactive.new; if somebody really really wants to
>    have different settings between diff/show/log, that person
>    could add color.{show,log}.new as well) are needed if we want
>    to do this.

Sure. Though like you said previously, I think in 99% of cases, nobody
is going to set these to something exotic; they're going to set them all
ot their custom colors, or all to "plain". In which case, we are just
making work for them by not providing "color.show = false".

> I am trying to avoid introducing new intermediate level knobs
> (e.g. color.log vs color.diff), as it is enough to disable or in
> general change the way particular parts of the UI is coloured by
> palette setting that specifically states which part of the UI is
> tweaked (e.g. color.interactive.prompt).

It is "enough" but I don't think it is the sensible goal. I respect your
desire to keep knobs to a minimum. But I think the fact that we can
split these color knobs into "dead simple, color = true" and "advanced,
super-picky color options" in the documentation makes it less of an
issue.

Anyway, almost none of this is pre-1.5.4. The one thing which is, IMHO,
is the semantics of color.interactive (since it has not yet been
released, this is the best time to set it). I will summarize my stance,
and you can do what you will. Beyond that, this discussion has already
taken way more time than this change is worth.

My opinion is that color.diff should be respected in "git-add -i"
regardless of color.interactive because:
  - the color.diff setting controls diffs in all porcelain parts of git
    (diff, log, show, reflog)
  - parts of diffs are the same as diffs. Thus, hunks should in
    general be treated like diffs for presentation.
  - splitting color by functionality (i.e., diff versus menus) is
    therefore more consistent with the rest of git than splitting it by
    program ("git add -i" versus "git diff")
  - I also happen to think that users are more likely to find "split by
    functionality" useful, but neither of us has relevant data
    (except that you seemed to indicate that's what you would find more
    useful, and that's what I find more useful. So it's 2-0, and clearly
    statistically significant :) ).

So if you are swayed by that, then please apply my previous patch, and
consider the documentation patch below. And if not, let me know and I
will begin changing my color.interactive.* config. :)

-- >8 --
Clarify color.interactive documentation

There are two possible confusions with the color.interactive
description:

  1. the short name "interactive" implies that it covers all
     interactive commands; let's explicitly make it so, even
     though there are no other interactive commands which
     currently use it

  2. Not all parts of "git add --interactive" are controlled
     by color.interactive (specifically, the diffs require
     tweaking color.diff). So let's clarify that it applies
     only to displays and prompts.

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/config.txt |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index e1eaee9..8907e16 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -406,7 +406,8 @@ color.diff.<slot>::
 	in color.branch.<slot>.
 
 color.interactive::
-	When set to `always`, always use colors in `git add --interactive`.
+	When set to `always`, always use colors for interactive prompts
+	and displays (such as those used by "git add --interactive").
 	When false (or `never`), never.  When set to `true` or `auto`, use
 	colors only when the output is to the terminal. Defaults to false.
 
-- 
1.5.4.rc2.1125.ga305e-dirty

^ permalink raw reply related

* gitk: possible bug in diffcmd
From: Maik Beckmann @ 2008-01-05  9:41 UTC (permalink / raw)
  To: git

Hello.  

I'm using git-1.5.3.7 on linux.  

When I try get use "Make Patch" from by the context menu and click on "Generate"
an error popup appears which states: 
  "Error creating patch: illegal use of | or &|"
I tracked the problem down to line 5064 in gitk, which belongs to diffcmd: 
  set cmd [concat | git diff-tree -r $flags $ids]  
This line produces something like 
  | git diff-tree -r -p 5441937e2... 7394db226...  
As you see there is a leading pipe symbol which caused the error message.  
Is there some special shell to use with gitk?? Or is it just a BUG?

If I remove the | after concat everything works smooth.

Thanks in advance,

 -- Maik

^ permalink raw reply

* [PATCH] git-stash clear: refuse to work with extra parameter for now
From: Junio C Hamano @ 2008-01-05  9:35 UTC (permalink / raw)
  To: JM Ibanez; +Cc: Jeff King, Brandon Casey, Git Mailing List
In-Reply-To: <871w8woc77.fsf@adler.orangeandbronze.com>

Because it is so tempting to expect "git stash clear stash@{4}"
to remove the fourth element in the stash while leaving other
elements intact, we should not blindly throw away everything.

This may change when we start using "git reflog delete" to
selectively nuke a single (or multiple, for that matter) stash
entries when such a command is given.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

JM Ibanez <jm@orangeandbronze.com> writes:

> Jeff King <peff@peff.net> writes:
> ...
>> 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 actually got hit by this. I didn't know that stash clear affected all
> stashes and lost quite a bit of work that way (I use stash to store
> various test database configs for a tree I work with, and so lost all of
> them when trying to remove one particular stash).

I think something along this line may be necessary to
futureproof our users. 

 git-stash.sh |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index 06cb177..80036ef 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -20,6 +20,10 @@ no_changes () {
 }
 
 clear_stash () {
+	if test $# != 0
+	then
+		die "git stash clear with parameters unimplemented $@"
+	fi
 	if current=$(git rev-parse --verify $ref_stash 2>/dev/null)
 	then
 		git update-ref -d $ref_stash $current
@@ -216,7 +220,7 @@ apply)
 	apply_stash "$@"
 	;;
 clear)
-	clear_stash
+	clear_stash "$@"
 	;;
 create)
 	if test $# -gt 0 && test "$1" = create

^ permalink raw reply related

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

Jeff King <peff@peff.net> writes:

> But he doesn't have to care about that. He cares only that "color.diff"
> means "diffs are displayed in color." And "color.interactive" means
> "interactive menus are displayed in color."
> ...
>> If he told "git add -p" to be monochrome, he has every right to expect
>> the part to pick hunks to also stay monochrome.  To people who know
>
> But he didn't. He said "git menus should be monochrome."

Who said anything about "interactive is limited to interactive
menus" anywhere?  That is where we differ and what you do not
seem to be getting.  I am talking about color.interactive that
controls the whole user experience of interacting with "add -i".

> Moreover, this doesn't allow "I always want color in diffs,
> but I don't want menu coloring" which is the very thing I have
> been trying to accomplish (but yes, I can do that by
> individually setting color.interactive.* to plain).

As you said earlier you may also be minority, but yes the color
pallette would help you do that.

> I fail to see how this is less confusing than just adding a separate
> interactive-diff knob, since you are asking them to individually set
> each color preference to plain.

What I am aiming at in longer term is to simplify things this way:

 * Users are categorized broadly into two groups.  The ones who
   like colours and the ones who don't want colours at all.
   color.git would control this (with backward compatibility
   options per command such as color.diff and
   color.interactive);

 * Minorities who want to disable colours for particular parts
   of the UI have enough knobs to tweak in the form of palettes.
   By definition this needs to address "particular parts", so
   "color.$command.$context" variables (e.g. color.diff.new,
   color.interactive.new; if somebody really really wants to
   have different settings between diff/show/log, that person
   could add color.{show,log}.new as well) are needed if we want
   to do this.

> E.g., given
> config options:
> ...
>> Admittedly, it's more work.
>
> Of course. ;) But I am willing to implement what I said above if you
> agree that it is sensible.

I think we share the ultimate goal of introducing higher level
knobs and our difference is just about minor details of how to
get there and what the intermediate levels look like.

I am trying to avoid introducing new intermediate level knobs
(e.g. color.log vs color.diff), as it is enough to disable or in
general change the way particular parts of the UI is coloured by
palette setting that specifically states which part of the UI is
tweaked (e.g. color.interactive.prompt).

^ permalink raw reply

* Re: [PATCH] git-stash: add new 'drop' subcommand
From: JM Ibanez @ 2008-01-05  9:26 UTC (permalink / raw)
  To: Jeff King; +Cc: Brandon Casey, Git Mailing List, Junio C Hamano
In-Reply-To: <20080105035118.GB26892@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> 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 actually got hit by this. I didn't know that stash clear affected all
stashes and lost quite a bit of work that way (I use stash to store
various test database configs for a tree I work with, and so lost all of
them when trying to remove one particular stash).

> 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.

I have to agree with this.

-- 
JM Ibanez
Software Architect
Orange & Bronze Software Labs, Ltd. Co.

jm@orangeandbronze.com
http://software.orangeandbronze.com/

^ permalink raw reply

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

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

> I understand your point. The problem is that in git there isn't an
> unique way to test success/fail for any command, as example, regarding
> checking the exit status:
>
> $ git status; echo $?
> # On branch master
> nothing to commit (working directory clean)
> 1

That is a bad example, with a slight historical background.

When you say "git status $args", you are asking the command this
question.

	I am contemplating to issue "git commit $args", but will
	there actually be changes if I issued that command?

When there will be no changes staged with the given $args (in
your case that happens to be empty), there won't be anything to
be committed if you issued "git commit $args" at that point.
The command answers "Eh, by issuing 'git commit' you will get
an 'Nothing to commit', which is an error" --- and that is
reported with its exit status.

^ permalink raw reply

* Re: [PATCH] git stash: one bug and one feature request
From: Marco Costalba @ 2008-01-05  8:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vbq80d5yp.fsf@gitster.siamese.dyndns.org>

On Jan 5, 2008 9:36 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> IOW, I do have much less objections to what your patch actually
> does, than I have problems with the way the reason for the
> change is stated.  The change is not fixing anything to conform
> to some standard behaviour.  It is more about bending
> (admittedly only slightly) backwards to help broken callers.
> That is what I have most trouble with.
>
>

Thanks for your understanding.

--------------------- CUT -----------------------------------

Subject: [PATCH] git-stash: use stdout instead of stderr for not error messages

Some scripts/libraries commonly check stderr to detect a
failing command. This is not standard nor good behaviour but
is quite common and in this case the change does not seem to hurt.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>
---
 git-stash.sh |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index 06cb177..4d5e5c0 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -86,7 +86,7 @@ save_stash () {

 	if no_changes
 	then
-		echo >&2 'No local changes to save'
+		echo 'No local changes to save'
 		exit 0
 	fi
 	test -f "$GIT_DIR/logs/$ref_stash" ||
@@ -99,7 +99,7 @@ save_stash () {

 	git update-ref -m "$stash_msg" $ref_stash $w_commit ||
 		die "Cannot save the current status"
-	printf >&2 'Saved working directory and index state "%s"\n' "$stash_msg"
+	printf 'Saved working directory and index state "%s"\n' "$stash_msg"
 }

 have_stash () {
@@ -229,7 +229,7 @@ create)
 	if test $# -eq 0
 	then
 		save_stash &&
-		echo >&2 '(To restore them type "git stash apply")' &&
+		echo '(To restore them type "git stash apply")' &&
 		git-reset --hard
 	else
 		usage
-- 
1.5.4.rc2.18.g530e6-dirty

^ permalink raw reply related

* Multiple shared repositories (including svn) workflow
From: Luke Lu @ 2008-01-05  8:54 UTC (permalink / raw)
  To: git

We have an existing project using subversion. Half of the committers  
want to use git for its features and speed, another half (mostly  
editors, graphic designers etc.) want to continue use svn for the  
usual reasons -- advanced features of git doesn't really buy them  
much. They just want to work as usual, especially with the ease of  
TortoiseSVN (some swear by it). After some thought and experiment, I  
proposed the following workflow:

0. People who want to use svn can continue using svn as usual.

1. A maintainer uses git-svn to convert the subversion project into git:
    git svn clone -s <project_url> project

2. He then creates a bare git repo as the "current" or "official"  
tree for the brave git users.
    git clone --bare project project.git
    project.git is then hosted on server, say, scm

3. Git users clone the "current" repo
    git clone git://scm/repos/project.git
    and work merrily ever after.

4. A maintainer (people or a cron job) would keep both official trees  
synced:
    a. He needs to add a git remote for the git-svn repo created at  
step 1:
       git remote add project ssh://scm/repos/project.git
    b. To sync commits from git to svn:
       git fetch && git rebase project/master
       git svn rebase
       git svn dcommit
    c. To sync from svn to git:
       git svn rebase
       git rebase project/master
       git push project

Preliminary tests showed that it seems to work well. Any problems  
that I didn't foresee?

Anyway, I've been using git for only a few months. There might/must  
be better ways to do it. As a user coming from cvs and svn, it seems  
to me that the most confusing command is actually "git pull" as it  
doesn't work with such workflow at all (conflicts with confusing  
messages, until you really understand the implications of rebase). It  
seems to me that if we create a new "git update" command which is  
essentially "git fetch && git rebase <remotebranch>", it would  
greatly alleviate such confusions. Some lessons learned from the  
short experience:

1. commit first, ask question later.
2. rebase is the new update
3. reset --hard and reflog are your friends.

Again, comments welcome.

__Luke

^ permalink raw reply

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

On Sat, Jan 05, 2008 at 12:01:51AM -0800, Junio C Hamano wrote:

> I think I understood that part.  What I was saying was that it
> is equally valid for other people to say "I like to interact
> with 'git add -i' without colours because coloured output
> distracts me when I have to think, even though I usually want to
> view the whole diff in colours."  So yes, color.interactive-diff

Right. My contention is that I think such people will be in the minority
(though obviously I have no numbers to back it up, and nobody else is
participating in this thread).

> To an end user, the fact that "git add -p" shows diff using the
> underlying "git diff" machinery does not matter.  That's just an
> implementation detail.

But he doesn't have to care about that. He cares only that "color.diff"
means "diffs are displayed in color." And "color.interactive" means
"interactive menus are displayed in color." Period. There is no concern
for the implementation detail of the diff machinery.

> "git diff" shows the whole diff at once while "git add -p" shows it
> hunk by hunk.  It is clear they are doing different things to the end
> user.

I don't see that distinction as relevant. Diffs are diffs, whether you
look at them with a small viewport or the whole thing. Would you expect
"git diff -- file1 file2" to have different display options than "git
diff -- file"?

> If he told "git add -p" to be monochrome, he has every right to expect
> the part to pick hunks to also stay monochrome.  To people who know

But he didn't. He said "git menus should be monochrome."

And yes, that's not exactly what config.txt says that color.interactive
does; but I think that is probably worth fixing.

> picker.  To others, it is counterintuitive if color.diff had any
> effect to what "git add -i" did.

Then why does it affect what "git log" does? Why does it affect what
"git reflog" does? Why does the documentation for color.diff say "use
colors in patch" without any reference to specific programs?

> Perhaps a saner alternative would be:
> 
>  * When color.interactive tells to use color, all interaction
>    with "add -i" will be in color.  There is no need to have
>    both color.diff and color.interactive set.
> 
>  * When color.interactive tells not to use color, everything
>    including the diff output will be monochrome.  What you have
>    in color.diff does not matter.

I think this is equally confusing. A user who sets color.diff will see
color diffs from all porcelains _except_ "add -i".

Moreover, this doesn't allow "I always want color in diffs, but I don't
want menu coloring" which is the very thing I have been trying to
accomplish (but yes, I can do that by individually setting
color.interactive.* to plain).

> The point of the third item is that you enable color.interactive
> and set diff related entries of color.interactive.* palette to
> plain, if you want some color while interacting with "add -i"
> but do not like colored hunk picker.  This would parallel the
> way you can selectively enable coloring in "git diff" output,
> where you enable color.diff and set metainfo color to plain if
> you want some color in diff output but do not like colored
> metainfo.

I fail to see how this is less confusing than just adding a separate
interactive-diff knob, since you are asking them to individually set
each color preference to plain. I.e., "set color.interactive to true and
color.interactive-diff to false" versus "set color.interactive to true,
but then for every type of diff colorization, set the color for it in
color.interactive.* to false".

I think the extra knob is not a problem; it is simply a matter of
defaulting the knobs based on other knobs. The rules for knobs
interacting may seem complex, but I think we can DWIM. E.g., given
config options:

  - color
  - color.diff
  - color.interactive
  - color.interactive.diff

The rules for "git add -i" diff coloring would be:

  (1) if color.interactive.diff is set to TRUE/FALSE, use that
  (2) otherwise, if color.interactive is set to TRUE/FALSE, use that
  (3) otherwise, if color.diff is set to TRUE/FALSE, use that
  (4) otherwise, if color is set to TRUE/FALSE, use that

IOW, even though we _have_ all of those knobs, users which don't want to
fine-tune can just use the higher-level knobs. Those that want to can
negate the higher level with lower level knobs. It's flexible, and it's
easy to tell users "just do 'git config color true'."

> Admittedly, it's more work.

Of course. ;) But I am willing to implement what I said above if you
agree that it is sensible.

-Peff

^ permalink raw reply

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

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

> This low level run() should know nothing about the semantic of the
> command or the outputted data, but should detect command failing,
> because failing reporting framework is unified and is the same for
> each type of command.

That sounds like a framework generalized in a wrong way to me.

> Please note that also gitk uses the same approach, indeed from
> http://ftp.tcl.tk/man/tcl8.5/tutorial/Tcl26.html you can read:
> ...

Heh, as if tcl is a textbook of good programming style.

> I can also black list not commonly behaving programs, but in case of
> git-stash a fail to see why to choose a not standard behaviour when
> not needed.

I do not offhand see a reason it would _hurt_ for this
particular case (git-stash) to write the diagnostics we
currently spit out to stderr to stdout.  My objection is
primarily because I do not think "never writing to stderr if
there is no error" is standard behaviour AT ALL.

IOW, I do have much less objections to what your patch actually
does, than I have problems with the way the reason for the
change is stated.  The change is not fixing anything to conform
to some standard behaviour.  It is more about bending
(admittedly only slightly) backwards to help broken callers.
That is what I have most trouble with.

^ permalink raw reply

* Re: [PATCH] git stash: one bug and one feature request
From: Marco Costalba @ 2008-01-05  8:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Wayne Davison, Git Mailing List
In-Reply-To: <7vzlvkepd5.fsf@gitster.siamese.dyndns.org>

On Jan 5, 2008 7:52 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Wayne Davison <wayne@opencoder.net> writes:
>
> > On Fri, Jan 04, 2008 at 05:14:42PM +0100, Marco Costalba wrote:
> >> -            echo >&2 'No local changes to save'
> >> +            echo > 'No local changes to save'
> >
> > That change and the other two following it each put a newline in a
> > strangely named file.  You should just drop the >&2 altogether if you
> > want the output to go to stdout.
>
> Lol...  Good eyes.  I did not even notice it ;-).
>

Very sorry for this, I should have been more careful. Please let me
know if you want me to resend the patch.

Marco

^ permalink raw reply

* Re: [PATCH] git stash: one bug and one feature request
From: Marco Costalba @ 2008-01-05  8:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vy7b5glmr.fsf@gitster.siamese.dyndns.org>

On Jan 5, 2008 1:29 AM, Junio C Hamano <gitster@pobox.com> wrote:
> "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).
>

I understand your point. The problem is that in git there isn't an
unique way to test success/fail for any command, as example, regarding
checking the exit status:

$ git status; echo $?
# On branch master
nothing to commit (working directory clean)
1


You get a value different from zero also in case of no error. The
checking for stderr I have found is more reliable for the git
command/scripts I use.

> 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.
>

I agree with this, but I fail to see the machine parsable output and
human consumption sideband info in case of git-stash that I would say
does not foreseen machine  parsable output at all, so in this case
choice of writing to stderr is less clear to me.

> 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.
>

If we don't want to have general rule for exit status and stderr at
least we could add a -q option to git stash, altough I would prefer
git stash writing on stdout if is not an error.

Please let me explain again why I need a reliable way to detect
success/fail of a command. When a function wants to execute a git
command it passes a string with the command + arguments to a low level
routine, say run(), that is command agnostic. This run() function
adapts and formats the command line according to the OS environment
then runs the command, saves the results and check for an error, the
result buffer is then passed as is to the caller that has the semantic
knowledge of what the command have produced.

This low level run() should know nothing about the semantic of the
command or the outputted data, but should detect command failing,
because failing reporting framework is unified and is the same for
each type of command.

A good and reliable way is to check for stderr, because it happens to
be more reliable then exit codes.

Please note that also gitk uses the same approach, indeed from
http://ftp.tcl.tk/man/tcl8.5/tutorial/Tcl26.html you can read:

--------------------

The 'exec' treats any output to standard error to be an indication
that the external program failed. This is simply a conservative
assumption: many programs behave that way and they are sloppy in
setting return codes.

Some programs however write to standard error without intending this
as an indication of an error. You can guard against this from
upsetting your script by using the catch command:

if { [catch { exec ls *.tcl } msg] } {
   puts "Something seems to have gone wrong but we will ignore it"
}

-------------------------

Indeed in gitk you find something like

    # Unfortunately git-cherry-pick writes stuff to stderr even when
    # no error occurs, and exec takes that as an indication of error...
    if {[catch {exec sh -c "git cherry-pick -r $rowmenuid 2>&1"} err]} {
	notbusy cherrypick
	error_popup $err
	return
    }



I can also black list not commonly behaving programs, but in case of
git-stash a fail to see why to choose a not standard behaviour when
not needed.


Thanks
Marco

^ permalink raw reply

* Re: [PATCH] t/t7600: avoid GNUism in grep
From: Junio C Hamano @ 2008-01-05  8:02 UTC (permalink / raw)
  To: Charles Bailey; +Cc: Miklos Vajna, git
In-Reply-To: <477F2E41.60105@hashpling.org>

Thanks.  Embarrasing.

^ permalink raw reply

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

Jeff King <peff@peff.net> writes:

> 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.

I think I understood that part.  What I was saying was that it
is equally valid for other people to say "I like to interact
with 'git add -i' without colours because coloured output
distracts me when I have to think, even though I usually want to
view the whole diff in colours."  So yes, color.interactive-diff
is an option to give more flexibility, but no I do not think
that's a good flexibility.  I'd prefer a single "color.git"
environment that rules all, which is far simpler to explain and
configure.  Either you use colour for all your interaction with
git, or you live in black-and-white world.

> [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).

And my point was that I doubt the change is such a big
improvement, certainly not in the way your justification
claimed, _even though I agree_ that the current "diff &&
interactive" way may not be something many people would want (by
the way, it happens to cover my preference, but that only means
I am a minority).  Neither covers all cases, and as I said, I do
not think more flexibility to cover all cases is necessarily a
good thing to shoot for.

Also there is another confusion factor we haven't discussed.

To an end user, the fact that "git add -p" shows diff using the
underlying "git diff" machinery does not matter.  That's just an
implementation detail.  "git diff" shows the whole diff at once
while "git add -p" shows it hunk by hunk.  It is clear they are
doing different things to the end user.  If he told "git add -p"
to be monochrome, he has every right to expect the part to pick
hunks to also stay monochrome.  To people who know the internal
implementation, it might be natural to expect the color.diff
configuration variable to affect the colouring of the hunk
picker.  To others, it is counterintuitive if color.diff had any
effect to what "git add -i" did.

> 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).

Yes, I 100% agree with you that they are confusing, and being to
able to futz with color.interactive.* palette would probably
alleviate the need to have color.{diff,interactive,status,etc}.

> ... 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 point I would agree with you but only 70%.  No sane user
would want deleted lines in red in "git diff" output and in
purple in hunk picker.  But I think it is reasonable to want to
view them in monochrome while in hunk picker (by setting
color.*.old to plain) but in red in normal diff output.

Perhaps a saner alternative would be:

 * When color.interactive tells to use color, all interaction
   with "add -i" will be in color.  There is no need to have
   both color.diff and color.interactive set.

 * When color.interactive tells not to use color, everything
   including the diff output will be monochrome.  What you have
   in color.diff does not matter.

 * We could allow color.interactive.* pallete to have elements
   that are parallel to color.diff.* palette to be used while
   showing the colored diff.  But this would be a low priority
   because (1) a custom setting to anything but "plain" does not
   make much practical sense, as it is just introducing needless
   inconsistency between "git diff" and the hunk picker, and (2)
   custom setting to "plain" would be used by people who like
   colored "git add -i" prompts but not colored hunk picker,
   which would be a minority.

The point of the third item is that you enable color.interactive
and set diff related entries of color.interactive.* palette to
plain, if you want some color while interacting with "add -i"
but do not like colored hunk picker.  This would parallel the
way you can selectively enable coloring in "git diff" output,
where you enable color.diff and set metainfo color to plain if
you want some color in diff output but do not like colored
metainfo.

Admittedly, it's more work.

^ permalink raw reply

* Re: [PATCH] t/t7600: avoid GNUism in grep
From: Charles Bailey @ 2008-01-05  7:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Miklos Vajna, git
In-Reply-To: <7v63y8g40u.fsf@gitster.siamese.dyndns.org>

Junio C Hamano wrote:
> diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
> index 6424c6e..21862ac 100755
> --- a/t/t7600-merge.sh
> +++ b/t/t7600-merge.sh
> @@ -371,7 +371,7 @@ test_expect_success 'override config option -n' '
>  	git merge --summary c2 >diffstat.txt &&
>  	verify_merge file result.1-5 msg.1-5 &&
>  	verify_parents $c1 $c2 &&
> -	if ! grep -e "^ file | \+2 +-$" diffstat.txt
> +	if ! grep -e "^ file |  *2 +-$" diffstat.txt
>  	then
>  		echo "[OOPS] diffstat was not generated"
>  	fi
> @@ -386,7 +386,7 @@ test_expect_success 'override config option --summary' '
>  	git merge -n c2 >diffstat.txt &&
>  	verify_merge file result.1-5 msg.1-5 &&
>  	verify_parents $c1 $c2 &&
> -	if grep -e "^ file | \+2 +-$" diffstat.txt
> +	if grep -e "^ file |  +2 +-$" diffstat.txt
>  	then
>  		echo "[OOPS] diffstat was generated"
>  		false

I'm not sure that you quite made the change you meant to change in the 
second chunk ( '  +' vs. '  *' ? ).

Charles.

^ permalink raw reply

* Re: [PATCH] git-am: Run git gc only once and not for every patch.
From: Junio C Hamano @ 2008-01-05  6:55 UTC (permalink / raw)
  To: Michael Stefaniuc; +Cc: Nicolas Pitre, git
In-Reply-To: <7vmyrli73h.fsf@gitster.siamese.dyndns.org>

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

> ...
> I do not think moving "gc --auto" outside the loop hurts in
> practice because you are not likely to be rebasing a truly huge
> series every day, but cruft can accumulate during "git am" run
> and the "gc --auto" inside loop was meant to clean them up.  The
> idea was taken from importers that run repack every once in a
> while (e.g. cvsimport runs every 1k commits), but "gc --auto"
> was designed to be much more lightweight than a full repack and
> that was the reason it was placed in the loop without counting
> "every N commits".

Having said all that, I'll take your patch as-is except that I'd
drop the later part of the commit log message that explains
Wine's practice.

^ permalink raw reply

* Re: [PATCH] git stash: one bug and one feature request
From: Junio C Hamano @ 2008-01-05  6:52 UTC (permalink / raw)
  To: Wayne Davison; +Cc: Marco Costalba, Git Mailing List
In-Reply-To: <20080105064156.GA6954@blorf.net>

Wayne Davison <wayne@opencoder.net> writes:

> On Fri, Jan 04, 2008 at 05:14:42PM +0100, Marco Costalba wrote:
>> -		echo >&2 'No local changes to save'
>> +		echo > 'No local changes to save'
>
> That change and the other two following it each put a newline in a
> strangely named file.  You should just drop the >&2 altogether if you
> want the output to go to stdout.

Lol...  Good eyes.  I did not even notice it ;-).

Thanks.

^ permalink raw reply

* [PATCH] t/t7600: avoid GNUism in grep
From: Junio C Hamano @ 2008-01-05  6:50 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: git
In-Reply-To: <1199506008-12225-1-git-send-email-vmiklos@frugalware.org>

Using \+ to mean "one or more" in grep without -E is a GNU
extension outside POSIX.  Avoid it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * I do not think these clean-ups are particularly necessary
   right now, but as we noticed it why not...

 t/t7600-merge.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 6424c6e..21862ac 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -371,7 +371,7 @@ test_expect_success 'override config option -n' '
 	git merge --summary c2 >diffstat.txt &&
 	verify_merge file result.1-5 msg.1-5 &&
 	verify_parents $c1 $c2 &&
-	if ! grep -e "^ file | \+2 +-$" diffstat.txt
+	if ! grep -e "^ file |  *2 +-$" diffstat.txt
 	then
 		echo "[OOPS] diffstat was not generated"
 	fi
@@ -386,7 +386,7 @@ test_expect_success 'override config option --summary' '
 	git merge -n c2 >diffstat.txt &&
 	verify_merge file result.1-5 msg.1-5 &&
 	verify_parents $c1 $c2 &&
-	if grep -e "^ file | \+2 +-$" diffstat.txt
+	if grep -e "^ file |  +2 +-$" diffstat.txt
 	then
 		echo "[OOPS] diffstat was generated"
 		false
-- 
1.5.4.rc2.17.g257f

^ permalink raw reply related

* [PATCH] t/t3800: do not use a temporary file to hold expected result.
From: Junio C Hamano @ 2008-01-05  6:48 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: git
In-Reply-To: <1199506008-12225-1-git-send-email-vmiklos@frugalware.org>

It is a good practice to write program output to a temporary file
during the test, as it would allow easier postmortem when the tested
program does break.  But there is no benefit in writing the expected
output out to the temporary.

This actually fixes a bug in check_verify_failure() routine.
The intention of the test seems to make sure the "git mktag" command
fails, and it spits out the expected error message.  But if the
command did not fail as expected, the shell function as originally
written would not have detected the failure.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t3800-mktag.sh |   89 +++++++++++++++--------------------------------------
 1 files changed, 25 insertions(+), 64 deletions(-)

diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
index e5f3073..f280320 100755
--- a/t/t3800-mktag.sh
+++ b/t/t3800-mktag.sh
@@ -12,10 +12,11 @@ test_description='git-mktag: tag object verify test'
 # given in the expect.pat file.
 
 check_verify_failure () {
-    test_expect_success \
-        "$1" \
-        'git-mktag <tag.sig 2>message ||
-       egrep -q -f expect.pat message'
+	expect="$2"
+	test_expect_success "$1" '
+		( ! git-mktag <tag.sig 2>message ) &&
+		grep -q "$expect" message
+	'
 }
 
 ###########################################################
@@ -33,11 +34,8 @@ cat >tag.sig <<EOF
 too short for a tag
 EOF
 
-cat >expect.pat <<EOF
-^error: .*size wrong.*$
-EOF
-
-check_verify_failure 'Tag object length check'
+check_verify_failure 'Tag object length check' \
+	'^error: .*size wrong.*$'
 
 ############################################################
 #  2. object line label check
@@ -48,11 +46,7 @@ type tag
 tag mytag
 EOF
 
-cat >expect.pat <<EOF
-^error: char0: .*"object "$
-EOF
-
-check_verify_failure '"object" line label check'
+check_verify_failure '"object" line label check' '^error: char0: .*"object "$'
 
 ############################################################
 #  3. object line SHA1 check
@@ -63,11 +57,7 @@ type tag
 tag mytag
 EOF
 
-cat >expect.pat <<EOF
-^error: char7: .*SHA1 hash$
-EOF
-
-check_verify_failure '"object" line SHA1 check'
+check_verify_failure '"object" line SHA1 check' '^error: char7: .*SHA1 hash$'
 
 ############################################################
 #  4. type line label check
@@ -78,11 +68,7 @@ xxxx tag
 tag mytag
 EOF
 
-cat >expect.pat <<EOF
-^error: char47: .*"[\]ntype "$
-EOF
-
-check_verify_failure '"type" line label check'
+check_verify_failure '"type" line label check' '^error: char47: .*"\\ntype "$'
 
 ############################################################
 #  5. type line eol check
@@ -90,11 +76,7 @@ check_verify_failure '"type" line label check'
 echo "object 779e9b33986b1c2670fff52c5067603117b3e895" >tag.sig
 printf "type tagsssssssssssssssssssssssssssssss" >>tag.sig
 
-cat >expect.pat <<EOF
-^error: char48: .*"[\]n"$
-EOF
-
-check_verify_failure '"type" line eol check'
+check_verify_failure '"type" line eol check' '^error: char48: .*"\\n"$'
 
 ############################################################
 #  6. tag line label check #1
@@ -105,11 +87,8 @@ type tag
 xxx mytag
 EOF
 
-cat >expect.pat <<EOF
-^error: char57: no "tag " found$
-EOF
-
-check_verify_failure '"tag" line label check #1'
+check_verify_failure '"tag" line label check #1' \
+	'^error: char57: no "tag " found$'
 
 ############################################################
 #  7. tag line label check #2
@@ -120,11 +99,8 @@ type taggggggggggggggggggggggggggggggg
 tag
 EOF
 
-cat >expect.pat <<EOF
-^error: char87: no "tag " found$
-EOF
-
-check_verify_failure '"tag" line label check #2'
+check_verify_failure '"tag" line label check #2' \
+	'^error: char87: no "tag " found$'
 
 ############################################################
 #  8. type line type-name length check
@@ -135,11 +111,8 @@ type taggggggggggggggggggggggggggggggg
 tag mytag
 EOF
 
-cat >expect.pat <<EOF
-^error: char53: type too long$
-EOF
-
-check_verify_failure '"type" line type-name length check'
+check_verify_failure '"type" line type-name length check' \
+	'^error: char53: type too long$'
 
 ############################################################
 #  9. verify object (SHA1/type) check
@@ -150,11 +123,8 @@ type tagggg
 tag mytag
 EOF
 
-cat >expect.pat <<EOF
-^error: char7: could not verify object.*$
-EOF
-
-check_verify_failure 'verify object (SHA1/type) check'
+check_verify_failure 'verify object (SHA1/type) check' \
+	'^error: char7: could not verify object.*$'
 
 ############################################################
 # 10. verify tag-name check
@@ -165,11 +135,8 @@ type commit
 tag my	tag
 EOF
 
-cat >expect.pat <<EOF
-^error: char67: could not verify tag name$
-EOF
-
-check_verify_failure 'verify tag-name check'
+check_verify_failure 'verify tag-name check' \
+	'^error: char67: could not verify tag name$'
 
 ############################################################
 # 11. tagger line label check #1
@@ -180,11 +147,8 @@ type commit
 tag mytag
 EOF
 
-cat >expect.pat <<EOF
-^error: char70: could not find "tagger"$
-EOF
-
-check_verify_failure '"tagger" line label check #1'
+check_verify_failure '"tagger" line label check #1' \
+	'^error: char70: could not find "tagger"$'
 
 ############################################################
 # 12. tagger line label check #2
@@ -196,11 +160,8 @@ tag mytag
 tagger
 EOF
 
-cat >expect.pat <<EOF
-^error: char70: could not find "tagger"$
-EOF
-
-check_verify_failure '"tagger" line label check #2'
+check_verify_failure '"tagger" line label check #2' \
+	'^error: char70: could not find "tagger"$'
 
 ############################################################
 # 13. create valid tag
-- 
1.5.4.rc2.17.g257f

^ permalink raw reply related

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

On Fri, Jan 04, 2008 at 05:14:42PM +0100, Marco Costalba wrote:
> -		echo >&2 'No local changes to save'
> +		echo > 'No local changes to save'

That change and the other two following it each put a newline in a
strangely named file.  You should just drop the >&2 altogether if you
want the output to go to stdout.

..wayne..

^ permalink raw reply

* Re: [PATCH] Document git-reset defaults to HEAD if no commit is given
From: Junio C Hamano @ 2008-01-05  6:01 UTC (permalink / raw)
  To: Marco Costalba; +Cc: Git Mailing List
In-Reply-To: <e5bfff550801040153m433d9880g4a7a6d4168d5365f@mail.gmail.com>

Thanks.

^ permalink raw reply

* Re: [PATCH] git-stash: add new 'drop' subcommand
From: Brandon Casey @ 2008-01-05  5:46 UTC (permalink / raw)
  To: git
In-Reply-To: <7vtzltf3gg.fsf@gitster.siamese.dyndns.org>

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

> 
> Brandon Casey <casey <at> 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?

yep that works. much nicer.

> > diff --git a/git-stash.sh b/git-stash.sh
> > -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.

no problem.


> > +	git reflog delete "$@" && echo "Dropped $@ ($s)" ||
> 
> The second $@ is inconsistent with the next line's use of $*; intentional?

not intentional.

> > +		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?

only because I didn't like stash@{0} notation and didn't want it
buried inside a function.

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

clear_stash additionally deletes refs/stash and logs/refs/stash at least.

-brandon

^ 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