* Re: [PATCH] bash-completion: don't add quoted space for ZSH (fix regression)
From: Junio C Hamano @ 2012-01-15 2:29 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git
In-Reply-To: <1326567336-2173-1-git-send-email-Matthieu.Moy@imag.fr>
Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index b0062ba..488e1f4 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -534,6 +534,12 @@ __gitcomp_nl ()
> fi
> fi
>
> + # ZSH would quote the trailing space added with -S. bash users
> + # will appreciate the extra space to compensate the use of -o nospace.
> + if [ -n "${ZSH_VERSION-}" ] && [ "$suffix" = " " ]; then
> + suffix=""
> + fi
This should take care of the SP set by the beginning of the helper
function
local cur_="$cur" suffix=" "
but is that the right thing to do if suffix came from "$4"?
As far as I can see, "$4" is used to append "." in very limited cases, and
nobody explicitly passes SP as "$4" when calling this, so it may be easier
to read if you moved this before that "if we have 3 or more args, use the
fourth one as the suffix" block, i.e. something like this?
contrib/completion/git-completion.bash | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index b0062ba..4ad75ed 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -525,7 +525,10 @@ __gitcomp ()
__gitcomp_nl ()
{
local s=$'\n' IFS=' '$'\t'$'\n'
- local cur_="$cur" suffix=" "
+ local cur_="$cur"
+ # Because we use '-o nospace' under bash, we need to compensate
+ # for it by appending SP after completed word ourselves.
+ local suffix="${BASH_VERSION+ }"
if [ $# -gt 2 ]; then
cur_="$3"
^ permalink raw reply related
* Re: Sending patches with KMail
From: Junio C Hamano @ 2012-01-15 2:14 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Rüdiger Sonderfeld, git, davidk, Sergei Organov, Kevin Ryde,
Michele Ballabio
In-Reply-To: <20120114183111.GC27850@burratino>
Jonathan Nieder <jrnieder@gmail.com> writes:
> My favorite approach would be to introduce a new option
> --format=plain|mbox, with the default being mbox, allowing format-patch
> --format=plain to produce a nice patch that does _not_ include a "From "
> line or q-encode its header lines, ready for use without much tweaking
> in an email body as an attachment.
I actually like the removal of q-encoding part. But I am not sure what
headers it should produce. What should the beginning of the output file
look like? Does it just have "Subject: ", or does it still have the "From:
", "Date: " and "Subject: ", the first two of which the user would almost
always want to remove?
If we can decide a sane behaviour wrt the pseudo header, and if the option
is made _incompatible_ with --stdout when (and only when) emitting more
than one message, then I think it would be a good addition.
^ permalink raw reply
* Re: git grep doesn't follow symbolic link
From: Junio C Hamano @ 2012-01-15 2:07 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy
Cc: Pang Yan Han, Thomas Rast, Ramkumar Ramachandra, Bertrand BENOIT,
git
In-Reply-To: <CACsJy8BfvhWxqBOj=+7AiF8dZBVEASAuxiOsjOvpmfE3uPrO3A@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>> Even though some platforms may lack symbolic links, where they are
>> supported, they have a clear and defined meaning and that is what Git
>> tracks as contents: where the link points at.
>>
>> So we would want our "git diff" to tell us, even if you moved without
>> content modification the symbolic link target that lives somewhere on your
>> filesystem but is outside the control of Git, and updated a symbolic link
>> that is tracked by Git to point to a new location, that you updated the
>> link. On the other hand, if you did not update a tracked symbolic link,
>> even if the location the link points at that may or may not be under the
>> control of Git, we do not want "git diff" to show anything. As far as that
>> link is concerned, nothing has changed.
>>
>> Changing this would not be a fix; it would be butchering.
>
> That's a good default. But git should allow me to say "diff the files
> that symlinks point to". Link target is content from git perspective,
> not from user perspective.
>
> So instead changing the default behavior specifically for git-grep as
> Pang did, I think adding --follow-symlinks option, that could be
> passed to grep or any of diff family, would be a better approach.
Stop and think what "git diff --follow-symlinks v1.3.0 v1.7.0" should do
when these versions record a symbolic link, "from user perspective", if
the link points outside the tracked contents. Naturally, the users would
expect that the comparison is made between the contents of the file back
when v1.3.0 was tagged and the contents of the file (which may or may not
be the same path depending on the target of that symbolic link) back when
v1.7.0 was tagged.
But that is something that the user is *NOT* tracking with the system, and
hence something we cannot give the right answer. Your "--follow-symlinks"
option only encourages the *wrong* perception on the users' side, without
supporting what it appears to promise to the users. Why could it be an
improvement?
Compared to that, limiting the optional support for following symlinks
only in "--no-index" case, where the user explicitly asks us to look at
the data that is not managed by Git at all, makes more sense. At the
design level, I wouldn't be fundamentally opposed to a change to add an
optional "follow the symlink" feature only when "--no-index" is asked for.
I didn't look at the posted patch, so I do not know if it adds an optional
following or unconditionally makes us follow symbolic links, or if the
patch sensibly implements the feature, though. That is a separate issue.
^ permalink raw reply
* Re: Zsh completion regression
From: Stefan Haller @ 2012-01-14 21:36 UTC (permalink / raw)
To: SZEDER Gábor, Matthieu Moy; +Cc: git
In-Reply-To: <20120114132343.GW30469@goldbirke>
SZEDER Gábor <szeder@ira.uka.de> wrote:
> However.
>
> While playing around with zsh, I noticed that git completion works
> without even sourcing git's bash completion script. As it turned out,
> zsh ships its own git completion script[1], and from my cursory tests
> it seems to be quite capable: it supports commands, aliases, options,
> refs, ref1..ref2, config variables, ... and I also saw a __git_ps1()
> equivalent for zsh.
>
> So, is there any reason why you are still using git's bash completion
> under zsh (which has some quirks and lacks some features) instead of
> zsh's own? Perhaps it would make sense to point zsh users to zsh's
> git completion and drop zsh compatibility from git's bash completion.
> We did similar with vim config files: git included a vim syntax
> highlight config file for commit messages under contrib/vim/, but
> eventually we dropped it after vim started shipping more capable
> git-specific config files (for git config files, rebase instruction
> sheets, etc.).
Last time I compared the two, the bash completion script was a lot more
complete and powerful. The zsh script had a few annoying limitations for
things that I use every day, and that worked with the bash script, so I
switched to that. Unfortunately I forgot the details, it's a while ago.
--
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/
^ permalink raw reply
* Re: Zsh completion regression
From: Stefan Haller @ 2012-01-14 21:36 UTC (permalink / raw)
To: SZEDER Gábor, Matthieu Moy; +Cc: git
In-Reply-To: <20120114152030.GX30469@goldbirke>
SZEDER Gábor <szeder@ira.uka.de> wrote:
> I can imagine that it's irritating, that's why I aimed for
> this minimal regression fix, which, maybe with a Tested-by from you or
> Stefan, can even go into maint, so affected users can get it faster.
The patch works well for me, so feel free to add a Tested-by from me.
--
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/
^ permalink raw reply
* Signed tags in octopus merge..
From: Linus Torvalds @ 2012-01-14 21:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
Just a heads-up and congrats: octopus merges of signed tags work well,
and did exactly the RightThing(tm), both at merge time and with
"--show-signature".
I knew it was supposed to work, but I have to admit that I was a bit
apprehensive about it when I tried.
Current top-of-head (commit 81d48f0aee54) in the kernel, in case you care.
Good job,
Linus
^ permalink raw reply
* The shared Git repo used by git-new-workdir
From: Hilco Wijbenga @ 2012-01-14 20:59 UTC (permalink / raw)
To: Git Users
Hi all,
First off, I use git-new-workdir a lot and it's working great. Kudos
to its developers!
I have been looking at the Git clone that is at the root of
git-new-workdir (i.e. the repository that is reused by all my
git-new-workdir created directories). This repo shows a lot of
activity when I run "git status" there.
So now I'm wondering. Should I simply ignore this completely? Or is
there some "clean up" I can do so that "git status" shows nothing? Or
would I destroy my git-new-workdir directories doing that? So far I've
only used this repo to create branches (i.e. to run git-new-workdir).
I would like to understand a bit better how I should treat this repo.
Whether it's basically a "do-not-touch" environment or whether I can
safely treat it as a normal Git repo.
Cheers,
Hilco
^ permalink raw reply
* Re: git-upload-archive help was not shown correctly
From: Jeff King @ 2012-01-14 20:56 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: devendra, git
In-Reply-To: <20120114154633.GA3444@beez.lab.cmartin.tk>
On Sat, Jan 14, 2012 at 04:46:33PM +0100, Carlos Martín Nieto wrote:
> On Sat, Jan 14, 2012 at 07:10:16PM +0530, devendra wrote:
> > Hi git folks,
> >
> > the command git-upload-archive is not properly showing usage info when
> > ran barely with out any args.
>
> git-upload-package is not for human use. It's what gets run on the
> remote end when you run e.g. 'git archive --remote=origin HEAD'
Yeah, but notice that it does detect the error condition; it just wraps
it in a bunch of cruft. More explanation (and a patch) are below, though
I'm lukewarm on the patch.
> > it shows some kind of unwanted garbage instead of showing a nice help
> > message.
>
> It's trying to talk to git. What you see is the "Git Smart
> Protocol". What were you trying to do?
Yeah, this is the more important question. You shouldn't generally ever
be running git-upload-* yourself.
-- >8 --
Subject: upload-archive: detect bad command-line arguments early
The upload-archive command spawns a helper process to
actually generate the archive (passing along all of its
command-line arguments), and the parent process then
multiplexes the stdout and stderr of the helper back to the
client.
This means an incorrect invocation (which in this case means
failing to provide a single <repo> argument) will cause the
helper to complain, and the error will be sent to the client
over the multiplexed channel.
This is not ideal for two reasons:
1. An invocation error in upload-pack is almost certainly
not something the client can do anything about. It
generally implies a bug in either git-daemon or
git-archive. The one exception is something like:
git archive --remote=ssh_host:repo.git \
--exec="git upload-archive bogus" \
2. For local users who are experimenting with git
commands, running "upload-archive" appears to generate
a bunch of barely-readable garbage (which is in fact
the git protocol intermingled with stderr). It's nicer
if we provide them with a readable error message.
Instead, we can detect a bad command-line before we spawn
the helper and give a more human-readable complaint (i.e.,
helping reason (2) above). This doesn't hurt case (1) as
much as you might think, because in the ssh case, the user
should be receiving stderr from the parent process anyway.
So before they saw:
$ git archive --remote=... --exec='git upload-archive bogus'
fatal: sent error to the client: git upload-archive: archiver died with error
remote: usage: git upload-archive <repo>
remote: git upload-archive: archiver died with error
and now they see:
usage: git upload-archive <repo>
fatal: The remote end hung up unexpectedly
Signed-off-by: Jeff King <peff@peff.net>
---
I'm lukewarm on this patch. I actually think the original output is
slightly nicer, so you are hurting case (1) for people in case (2). But
case (2), running random programs to see what happens, is not something
we probably need to be encouraging.
builtin/upload-archive.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index b928beb..0733775 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -10,6 +10,8 @@
static const char upload_archive_usage[] =
"git upload-archive <repo>";
+static const char upload_archive_writer_usage[] =
+ "git upload-archive--writer <repo>";
static const char deadchild[] =
"git upload-archive: archiver died with error";
@@ -25,7 +27,7 @@ int cmd_upload_archive_writer(int argc, const char **argv, const char *prefix)
int len;
if (argc != 2)
- usage(upload_archive_usage);
+ usage(upload_archive_writer_usage);
if (strlen(argv[1]) + 1 > sizeof(buf))
die("insanely long repository name");
@@ -96,6 +98,9 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
{
struct child_process writer = { argv };
+ if (argc != 2)
+ usage(upload_archive_usage);
+
/*
* Set up sideband subprocess.
*
--
1.7.9.rc0.33.gd3c17
^ permalink raw reply related
* Re: Sending patches with KMail (Re: [PATCH] git-blame.el: Fix compilation warnings.)
From: Rüdiger Sonderfeld @ 2012-01-14 19:18 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, git, davidk, Sergei Organov, Kevin Ryde,
Michele Ballabio
In-Reply-To: <7vlipbxfne.fsf@alter.siamese.dyndns.org>
On Friday 13 January 2012 16:59:49 Junio C Hamano wrote:
> Perhaps rephrasing the early part of the Discussion section, with an
> illustration that is designed to be more visible, would be a better
> approach?
Why not do both?
I think you are right, that it is currently not very visible in the Discussion
section. But on the other hand if you have a step by step guide it should
probably mention that as well. It has nothing to do with laziness. But most
people follow a step by step guide because they expect that it illustrates the
correct procedure. Jonathan's addition is short but effective.
Regards,
Rüdiger
^ permalink raw reply
* [PATCH] bash-completion: don't add quoted space for ZSH (fix regression)
From: Matthieu Moy @ 2012-01-14 18:55 UTC (permalink / raw)
To: git, gitster; +Cc: Matthieu Moy
In-Reply-To: <20120114152030.GX30469@goldbirke>
Commit a31e626 (completion: optimize refs completion) introduced a
regression for ZSH users: ref names were completed with a quoted trailing
space (i.e. "git checkout ma" completes to "git checkout master\ "). The
space is convenient for bash users since we use "-o nospace", but a
quoted space is worse than nothing. The absence of trailing space for ZSH
is a long-standing issue, that this patch is not fixing. We just fix the
regression by not appending a space when the shell is ZSH.
Original-patch-by: SZEDER Gábor <szeder@ira.uka.de>
Reported-by: Stefan Haller <lists@haller-berlin.de>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
contrib/completion/git-completion.bash | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index b0062ba..488e1f4 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -534,6 +534,12 @@ __gitcomp_nl ()
fi
fi
+ # ZSH would quote the trailing space added with -S. bash users
+ # will appreciate the extra space to compensate the use of -o nospace.
+ if [ -n "${ZSH_VERSION-}" ] && [ "$suffix" = " " ]; then
+ suffix=""
+ fi
+
IFS=$s
COMPREPLY=($(compgen -P "${2-}" -S "$suffix" -W "$1" -- "$cur_"))
}
--
1.7.9.rc0.25.gb9a1f.dirty
^ permalink raw reply related
* Re: Sending patches with KMail
From: Jonathan Nieder @ 2012-01-14 18:34 UTC (permalink / raw)
To: Junio C Hamano
Cc: Rüdiger Sonderfeld, git, davidk, Sergei Organov, Kevin Ryde,
Michele Ballabio
In-Reply-To: <20120114183111.GC27850@burratino>
Jonathan Nieder wrote:
> My favorite approach would be to introduce a new option
> --format=plain|mbox, with the default being mbox, allowing
> format-patch --format=plain to produce a nice patch that does _not_
> include a "From " line or q-encode its header lines, ready for use
> without much tweaking in an email body as an attachment.
This should have said "ready for use in an email body or as an
attachment" (missing "or"). Sorry for the confusion.
^ permalink raw reply
* Re: Sending patches with KMail
From: Jonathan Nieder @ 2012-01-14 18:31 UTC (permalink / raw)
To: Junio C Hamano
Cc: Rüdiger Sonderfeld, git, davidk, Sergei Organov, Kevin Ryde,
Michele Ballabio
In-Reply-To: <7vlipbxfne.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>> The hints at [1] might also be useful, in case you would like to try
>> and consider improving the manpage to document them if they work.
>
> Don't you need similar updates to sections for other MUAs and procedures?
Thunderbird approach 3, yes[*]. The others, no.
[...]
> Perhaps rephrasing the early part of the Discussion section, with an
> illustration that is designed to be more visible, would be a better
> approach?
I understand what you mean, but I don't think so. The Discussion
section already seems clear to me, so I would prefer to wait to hear
from someone confused by it to find what exactly in it needs tweaking.
Adding additional paragraphs for each potential misunderstanding by
people who have not necessarily read the section has the potential to
backfire and lead even more people not to read the section...
My favorite approach would be to introduce a new option
--format=plain|mbox, with the default being mbox, allowing
format-patch --format=plain to produce a nice patch that does _not_
include a "From " line or q-encode its header lines, ready for use
without much tweaking in an email body as an attachment. Then we can
just say "If you are not importing your patch as an mbox file, use the
--format=plain option".
Sane?
Jonathan
[*] Though I'd rather just remove it, since "how to use an external
editor" seems orthogonal to "how to teach Thunderbird not to mangle my
patches".
^ permalink raw reply
* Re: git-upload-archive help was not shown correctly
From: Carlos Martín Nieto @ 2012-01-14 15:46 UTC (permalink / raw)
To: devendra; +Cc: git
In-Reply-To: <1326548416.5992.1.camel@devendra-Linux>
[-- Attachment #1: Type: text/plain, Size: 525 bytes --]
On Sat, Jan 14, 2012 at 07:10:16PM +0530, devendra wrote:
> Hi git folks,
>
> the command git-upload-archive is not properly showing usage info when
> ran barely with out any args.
git-upload-package is not for human use. It's what gets run on the
remote end when you run e.g. 'git archive --remote=origin HEAD'
>
> it shows some kind of unwanted garbage instead of showing a nice help
> message.
It's trying to talk to git. What you see is the "Git Smart
Protocol". What were you trying to do?
cmn
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/3] Eliminate recursion in setting/clearing marks in commit list
From: Peter Baumann @ 2012-01-14 15:23 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy
Cc: git, Junio C Hamano, Shawn O. Pearce
In-Reply-To: <1326543595-28300-2-git-send-email-pclouds@gmail.com>
On Sat, Jan 14, 2012 at 07:19:53PM +0700, Nguyễn Thái Ngọc Duy wrote:
> Recursion in a DAG is generally a bad idea because it could be very
> deep. Be defensive and avoid recursion in mark_parents_uninteresting()
> and clear_commit_marks().
>
> mark_parents_uninteresting() learns a trick from clear_commit_marks()
> to avoid malloc() in (dorminant) single-parent case.
^^^^^^^^^
I think you ment dominant here.
^ permalink raw reply
* Re: Zsh completion regression
From: SZEDER Gábor @ 2012-01-14 15:20 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Stefan Haller, git
In-Reply-To: <vpq7h0ufj87.fsf@bauges.imag.fr>
Hi,
On Sat, Jan 14, 2012 at 03:32:08PM +0100, Matthieu Moy wrote:
> SZEDER Gábor <szeder@ira.uka.de> writes:
> > We could fix the regression by not appending a space suffix to
> > completion words in __gitcomp_nl(), but only when the completion
> > script is running under zsh to avoid hurting bash users, like this:
> >
> > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> > index 2d02a7f3..49393243 100755
> > --- a/contrib/completion/git-completion.bash
> > +++ b/contrib/completion/git-completion.bash
> > @@ -601,6 +601,9 @@ __gitcomp_nl ()
> > suffix="$4"
> > fi
> > fi
> > + if [ -n "${ZSH_VERSION-}" ] && [ "$suffix" = " " ]; then
> > + suffix=""
> > + fi
> >
> > IFS=$s
> > COMPREPLY=($(compgen -P "${2-}" -S "$suffix" -W "$1" -- "$cur_"))
>
> I hate to see special case for different shells,
I agree and thought about that, too. We are dealing with zsh quirks
in the completion script either by
- having whole functions with different definitions in zsh and
in bash (currently__git_shopt() and _get_comp_words_by_ref()), or
- having similar shell-specific cases inside functions (currently
_git() and _gitk()).
We use the former when the implementations for the two shells differ
significantly, and the latter when the shell-specific parts are small
and most of the function's implementation can be used in both shells.
I think this regression fix fits into the latter category.
> but if no one finds a
> better solution, then yes, this is the way to go. Not having the space
> may be irritating, but having the quoted space hurts really much more (I
> have to delete the space and the backslash manually to continue).
I have no better idea for fixing this regression, and no idea at all
for a proper fix (i.e. to make zsh behave the same way in this respect
as bash). I can imagine that it's irritating, that's why I aimed for
this minimal regression fix, which, maybe with a Tested-by from you or
Stefan, can even go into maint, so affected users can get it faster.
Best,
Gábor
^ permalink raw reply
* Re: Zsh completion regression
From: Matthieu Moy @ 2012-01-14 14:32 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Stefan Haller, git
In-Reply-To: <20120114132343.GW30469@goldbirke>
SZEDER Gábor <szeder@ira.uka.de> writes:
>> but e.g. "git checkout
>> master<TAB>" does not add the trailing space, at all.
>
> I'm not sure what you mean; did you got a trailing space after
> 'master<TAB>' before a31e6262 (completion: optimize refs completion,
> 2011-10-15)?
No. My above sentence should read "... not add the trailing space, at
all, even for bash users". IOW, your understanding is correct.
> We could fix the regression by not appending a space suffix to
> completion words in __gitcomp_nl(), but only when the completion
> script is running under zsh to avoid hurting bash users, like this:
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 2d02a7f3..49393243 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -601,6 +601,9 @@ __gitcomp_nl ()
> suffix="$4"
> fi
> fi
> + if [ -n "${ZSH_VERSION-}" ] && [ "$suffix" = " " ]; then
> + suffix=""
> + fi
>
> IFS=$s
> COMPREPLY=($(compgen -P "${2-}" -S "$suffix" -W "$1" -- "$cur_"))
I hate to see special case for different shells, but if no one finds a
better solution, then yes, this is the way to go. Not having the space
may be irritating, but having the quoted space hurts really much more (I
have to delete the space and the backslash manually to continue).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: Commit changes to remote repository
From: Matthieu Moy @ 2012-01-14 14:27 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: ruperty, git
In-Reply-To: <20120114113141.GG2850@centaur.lab.cmartin.tk>
Carlos Martín Nieto <cmn@elego.de> writes:
> You're trying to push to a non-bare repository and change the
> currently active branch, which can cause problems, so git isn't
> letting you. There's an explanation of bare and non-bare at
> http://bare-vs-nonbare.gitrecipes.de/ but the short and sweet is that
> you should init the repo you want to use as the central point with
> --bare and do modifications locally and then push there.
An alternative is to push to a temporary, non-checked-out branch.
I sometimes do
laptop$ git push desktop HEAD:incomming
and then
desktop$ git merge incomming
The push does not disturb the worktree on the desktop, and the merge is
done manually on the receiving machine.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* git-upload-archive help was not shown correctly
From: devendra @ 2012-01-14 13:40 UTC (permalink / raw)
To: git
Hi git folks,
the command git-upload-archive is not properly showing usage info when
ran barely with out any args.
it shows some kind of unwanted garbage instead of showing a nice help
message.
The output is pasted here.
root@devendra-Linux:/home/devendra/git/Documentation#
git-upload-archive
0008ACK
00000026 usage: git upload-archive <repo>
0031 git upload-archive: archiver died with errorfatal: sent error to
the client: git upload-archive: archiver died with error
my git latest version shows commit sha1 number
6db5c6e43dccb380ca6e9947777985eb11248c31.
Devendra.
^ permalink raw reply
* Re: Zsh completion regression
From: SZEDER Gábor @ 2012-01-14 13:23 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Stefan Haller, git
In-Reply-To: <vpq62ghj7fn.fsf@bauges.imag.fr>
Hi,
On Thu, Jan 12, 2012 at 03:56:28PM +0100, Matthieu Moy wrote:
> lists@haller-berlin.de (Stefan Haller) writes:
>
> > I'm using zsh 4.3.11.
> >
> > When I type "git log mas<TAB>", it completes to "git log master\ " (a
> > backslash, a space, and then the cursor).
Stefan, thanks for reporting and bisecting.
> The following patch makes the situation better, but is not really a fix:
>
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -525,7 +525,7 @@ __gitcomp ()
> __gitcomp_nl ()
> {
> local s=$'\n' IFS=' '$'\t'$'\n'
> - local cur_="$cur" suffix=" "
> + local cur_="$cur" suffix=""
>
> if [ $# -gt 2 ]; then
> cur_="$3"
>
> With this, the trailing space isn't added,
Yeah, this would be a regression for Bash users, because then they
will need to manually append that space.
> but e.g. "git checkout
> master<TAB>" does not add the trailing space, at all.
I'm not sure what you mean; did you got a trailing space after
'master<TAB>' before a31e6262 (completion: optimize refs completion,
2011-10-15)?
I'm not a zsh user myself, but just tried it and found that in
a31e626^ 'git checkout master<TAB>' doesn't add the trailing space,
and neither does 'git checkout mas<TAB>', which is in sync with what
Stefan reported ("Before this commit, I get "git log master" (with no
space at the end).").
> The problem is a little bit below:
>
> IFS=$s
> COMPREPLY=($(compgen -P "${2-}" -S "$suffix" -W "$1" -- "$cur_"))
>
> The -S "$suffix" adds a space to the completion, but ZSH escapes the
> space (which sounds sensible in general, but is not at all what we
> expect).
So it seems we have two issues here with zsh:
- Spaces appended with 'compgen -S " "' in __gitcomp_nl() are quoted.
This is the regression caused by a31e6262.
- Spaces appended in __gitcomp_1() (by echo "$word ") are stripped.
This is a long-standing issue; if those spaces weren't stripped, the
quoting issue would have been noted earlier, I suspect.
We could fix the regression by not appending a space suffix to
completion words in __gitcomp_nl(), but only when the completion
script is running under zsh to avoid hurting bash users, like this:
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2d02a7f3..49393243 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -601,6 +601,9 @@ __gitcomp_nl ()
suffix="$4"
fi
fi
+ if [ -n "${ZSH_VERSION-}" ] && [ "$suffix" = " " ]; then
+ suffix=""
+ fi
IFS=$s
COMPREPLY=($(compgen -P "${2-}" -S "$suffix" -W "$1" -- "$cur_"))
Although it wouldn't append a space at all for zsh users, that's
nothing new, it's just restoring old behavior. And then later someone
more knowledgable about the quirks of zsh's completion and in
particular zsh's bash completion emulation can come up with a proper
fix to the issue of quoted spaces and stripped trailing spaces.
However.
While playing around with zsh, I noticed that git completion works
without even sourcing git's bash completion script. As it turned out,
zsh ships its own git completion script[1], and from my cursory tests
it seems to be quite capable: it supports commands, aliases, options,
refs, ref1..ref2, config variables, ... and I also saw a __git_ps1()
equivalent for zsh.
So, is there any reason why you are still using git's bash completion
under zsh (which has some quirks and lacks some features) instead of
zsh's own? Perhaps it would make sense to point zsh users to zsh's
git completion and drop zsh compatibility from git's bash completion.
We did similar with vim config files: git included a vim syntax
highlight config file for commit messages under contrib/vim/, but
eventually we dropped it after vim started shipping more capable
git-specific config files (for git config files, rebase instruction
sheets, etc.).
[1] http://zsh.git.sourceforge.net/git/gitweb.cgi?p=zsh/zsh;a=blob;f=Completion/Unix/Command/_git;hb=HEAD
Best,
Gábor
^ permalink raw reply related
* [PATCH v3 3/3] index-pack: eliminate unlimited recursion in get_base_data()
From: Nguyễn Thái Ngọc Duy @ 2012-01-14 12:19 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Shawn O. Pearce,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1326543595-28300-1-git-send-email-pclouds@gmail.com>
Revese the order of delta applying so that by the time a delta is
applied, its base is either non-delta or already inflated.
get_base_data() is still recursive, but because base's data is always
ready, the inner get_base_data() call never has any chance to call
itself again.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/index-pack.c | 53 +++++++++++++++++++++++++++++++++++++++++--------
1 files changed, 44 insertions(+), 9 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 38ff03a..dc6a584 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -515,14 +515,52 @@ static int is_delta_type(enum object_type type)
return (type == OBJ_REF_DELTA || type == OBJ_OFS_DELTA);
}
+/*
+ * This function is part of find_unresolved_deltas(). There are two
+ * walkers going in the opposite ways.
+ *
+ * The first one in find_unresolved_deltas() traverses down from
+ * parent node to children, deflating nodes along the way. However,
+ * memory for deflated nodes is limited by delta_base_cache_limit, so
+ * at some point parent node's deflated content may be freed.
+ *
+ * The second walker is this function, which goes from current node up
+ * to top parent if necessary to deflate the node. In normal
+ * situation, its parent node would be already deflated, so it just
+ * needs to apply delta.
+ *
+ * In worst case scenario, parent node is no longer deflated because
+ * we're running out of delta_base_cache_limit, then we need to
+ * re-deflate parents, possibly up to the top base.
+ *
+ * All deflated objecsts here are subject to be freed if we exceed
+ * delta_base_cache_limit, just like in find_unresolved_deltas(), we
+ * just need to make sure the last node is not freed.
+ */
static void *get_base_data(struct base_data *c)
{
if (!c->data) {
struct object_entry *obj = c->obj;
+ struct base_data **delta = NULL;
+ int delta_nr = 0, delta_alloc = 0;
- if (is_delta_type(obj->type)) {
- void *base = get_base_data(c->base);
- void *raw = get_data_from_pack(obj);
+ while (is_delta_type(c->obj->type) && !c->data) {
+ ALLOC_GROW(delta, delta_nr + 1, delta_alloc);
+ delta[delta_nr++] = c;
+ c = c->base;
+ }
+ if (!delta_nr) {
+ c->data = get_data_from_pack(obj);
+ c->size = obj->size;
+ base_cache_used += c->size;
+ prune_base_data(c);
+ }
+ for (; delta_nr > 0; delta_nr--) {
+ void *base, *raw;
+ c = delta[delta_nr - 1];
+ obj = c->obj;
+ base = get_base_data(c->base);
+ raw = get_data_from_pack(obj);
c->data = patch_delta(
base, c->base->size,
raw, obj->size,
@@ -530,13 +568,10 @@ static void *get_base_data(struct base_data *c)
free(raw);
if (!c->data)
bad_object(obj->idx.offset, "failed to apply delta");
- } else {
- c->data = get_data_from_pack(obj);
- c->size = obj->size;
+ base_cache_used += c->size;
+ prune_base_data(c);
}
-
- base_cache_used += c->size;
- prune_base_data(c);
+ free(delta);
}
return c->data;
}
--
1.7.8.36.g69ee2
^ permalink raw reply related
* [PATCH v3 1/3] Eliminate recursion in setting/clearing marks in commit list
From: Nguyễn Thái Ngọc Duy @ 2012-01-14 12:19 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Shawn O. Pearce,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1326543595-28300-1-git-send-email-pclouds@gmail.com>
Recursion in a DAG is generally a bad idea because it could be very
deep. Be defensive and avoid recursion in mark_parents_uninteresting()
and clear_commit_marks().
mark_parents_uninteresting() learns a trick from clear_commit_marks()
to avoid malloc() in (dorminant) single-parent case.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
commit.c | 13 +++++++++++--
revision.c | 45 +++++++++++++++++++++++++++++----------------
2 files changed, 40 insertions(+), 18 deletions(-)
diff --git a/commit.c b/commit.c
index 44bc96d..c7aefbf 100644
--- a/commit.c
+++ b/commit.c
@@ -421,7 +421,8 @@ struct commit *pop_most_recent_commit(struct commit_list **list,
return ret;
}
-void clear_commit_marks(struct commit *commit, unsigned int mark)
+static void clear_commit_marks_1(struct commit_list **plist,
+ struct commit *commit, unsigned int mark)
{
while (commit) {
struct commit_list *parents;
@@ -436,12 +437,20 @@ void clear_commit_marks(struct commit *commit, unsigned int mark)
return;
while ((parents = parents->next))
- clear_commit_marks(parents->item, mark);
+ commit_list_insert(parents->item, plist);
commit = commit->parents->item;
}
}
+void clear_commit_marks(struct commit *commit, unsigned int mark)
+{
+ struct commit_list *list = NULL;
+ commit_list_insert(commit, &list);
+ while (list)
+ clear_commit_marks_1(&list, pop_commit(&list), mark);
+}
+
void clear_commit_marks_for_object_array(struct object_array *a, unsigned mark)
{
struct object *object;
diff --git a/revision.c b/revision.c
index 8764dde..7cc72fc 100644
--- a/revision.c
+++ b/revision.c
@@ -139,11 +139,32 @@ void mark_tree_uninteresting(struct tree *tree)
void mark_parents_uninteresting(struct commit *commit)
{
- struct commit_list *parents = commit->parents;
+ struct commit_list *parents = NULL, *l;
+
+ for (l = commit->parents; l; l = l->next)
+ commit_list_insert(l->item, &parents);
while (parents) {
struct commit *commit = parents->item;
- if (!(commit->object.flags & UNINTERESTING)) {
+ l = parents;
+ parents = parents->next;
+ free(l);
+
+ while (commit) {
+ /*
+ * A missing commit is ok iff its parent is marked
+ * uninteresting.
+ *
+ * We just mark such a thing parsed, so that when
+ * it is popped next time around, we won't be trying
+ * to parse it and get an error.
+ */
+ if (!has_sha1_file(commit->object.sha1))
+ commit->object.parsed = 1;
+
+ if (commit->object.flags & UNINTERESTING)
+ break;
+
commit->object.flags |= UNINTERESTING;
/*
@@ -154,21 +175,13 @@ void mark_parents_uninteresting(struct commit *commit)
* wasn't uninteresting), in which case we need
* to mark its parents recursively too..
*/
- if (commit->parents)
- mark_parents_uninteresting(commit);
- }
+ if (!commit->parents)
+ break;
- /*
- * A missing commit is ok iff its parent is marked
- * uninteresting.
- *
- * We just mark such a thing parsed, so that when
- * it is popped next time around, we won't be trying
- * to parse it and get an error.
- */
- if (!has_sha1_file(commit->object.sha1))
- commit->object.parsed = 1;
- parents = parents->next;
+ for (l = commit->parents->next; l; l = l->next)
+ commit_list_insert(l->item, &parents);
+ commit = commit->parents->item;
+ }
}
}
--
1.7.8.36.g69ee2
^ permalink raw reply related
* [PATCH v3 2/3] index-pack: eliminate recursion in find_unresolved_deltas
From: Nguyễn Thái Ngọc Duy @ 2012-01-14 12:19 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Shawn O. Pearce,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1326543595-28300-1-git-send-email-pclouds@gmail.com>
Current find_unresolved_deltas() links all bases together in a form of
tree, using struct base_data, with prev_base pointer to point to
parent node. Then it traverses down from parent to children in
recursive manner with all base_data allocated on stack.
To eliminate recursion, we simply need to put all on heap
(parse_pack_objects and fix_unresolved_deltas). After that, it's
simple non-recursive depth-first traversal loop. Each node also
maintains its own state (ofs and ref indices) to iterate over all
children nodes.
So we process one node:
- if it returns a new (child) node (a parent base), we link it to our
tree, then process the new node.
- if it returns nothing, the node is done, free it. We go back to
parent node and resume whatever it's doing.
and do it until we have no nodes to process.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/index-pack.c | 111 +++++++++++++++++++++++++++++++------------------
1 files changed, 70 insertions(+), 41 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index af7dc37..38ff03a 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -34,6 +34,8 @@ struct base_data {
struct object_entry *obj;
void *data;
unsigned long size;
+ int ref_first, ref_last;
+ int ofs_first, ofs_last;
};
/*
@@ -221,6 +223,15 @@ static NORETURN void bad_object(unsigned long offset, const char *format, ...)
die("pack has bad object at offset %lu: %s", offset, buf);
}
+static struct base_data *alloc_base_data(void)
+{
+ struct base_data *base = xmalloc(sizeof(struct base_data));
+ memset(base, 0, sizeof(*base));
+ base->ref_last = -1;
+ base->ofs_last = -1;
+ return base;
+}
+
static void free_base_data(struct base_data *c)
{
if (c->data) {
@@ -553,58 +564,76 @@ static void resolve_delta(struct object_entry *delta_obj,
nr_resolved_deltas++;
}
-static void find_unresolved_deltas(struct base_data *base,
- struct base_data *prev_base)
+static struct base_data *find_unresolved_deltas_1(struct base_data *base,
+ struct base_data *prev_base)
{
- int i, ref_first, ref_last, ofs_first, ofs_last;
-
- /*
- * This is a recursive function. Those brackets should help reducing
- * stack usage by limiting the scope of the delta_base union.
- */
- {
+ if (base->ref_last == -1 && base->ofs_last == -1) {
union delta_base base_spec;
hashcpy(base_spec.sha1, base->obj->idx.sha1);
find_delta_children(&base_spec,
- &ref_first, &ref_last, OBJ_REF_DELTA);
+ &base->ref_first, &base->ref_last, OBJ_REF_DELTA);
memset(&base_spec, 0, sizeof(base_spec));
base_spec.offset = base->obj->idx.offset;
find_delta_children(&base_spec,
- &ofs_first, &ofs_last, OBJ_OFS_DELTA);
- }
+ &base->ofs_first, &base->ofs_last, OBJ_OFS_DELTA);
- if (ref_last == -1 && ofs_last == -1) {
- free(base->data);
- return;
- }
+ if (base->ref_last == -1 && base->ofs_last == -1) {
+ free(base->data);
+ return NULL;
+ }
- link_base_data(prev_base, base);
+ link_base_data(prev_base, base);
+ }
- for (i = ref_first; i <= ref_last; i++) {
- struct object_entry *child = objects + deltas[i].obj_no;
- struct base_data result;
+ if (base->ref_first <= base->ref_last) {
+ struct object_entry *child = objects + deltas[base->ref_first].obj_no;
+ struct base_data *result = alloc_base_data();
assert(child->real_type == OBJ_REF_DELTA);
- resolve_delta(child, base, &result);
- if (i == ref_last && ofs_last == -1)
+ resolve_delta(child, base, result);
+ if (base->ref_first == base->ref_last && base->ofs_last == -1)
free_base_data(base);
- find_unresolved_deltas(&result, base);
+
+ base->ref_first++;
+ return result;
}
- for (i = ofs_first; i <= ofs_last; i++) {
- struct object_entry *child = objects + deltas[i].obj_no;
- struct base_data result;
+ if (base->ofs_first <= base->ofs_last) {
+ struct object_entry *child = objects + deltas[base->ofs_first].obj_no;
+ struct base_data *result = alloc_base_data();
assert(child->real_type == OBJ_OFS_DELTA);
- resolve_delta(child, base, &result);
- if (i == ofs_last)
+ resolve_delta(child, base, result);
+ if (base->ofs_first == base->ofs_last)
free_base_data(base);
- find_unresolved_deltas(&result, base);
+
+ base->ofs_first++;
+ return result;
}
unlink_base_data(base);
+ return NULL;
+}
+
+static void find_unresolved_deltas(struct base_data *base)
+{
+ struct base_data *new_base, *prev_base = NULL;
+ for (;;) {
+ new_base = find_unresolved_deltas_1(base, prev_base);
+
+ if (new_base) {
+ prev_base = base;
+ base = new_base;
+ } else {
+ free(base);
+ base = prev_base;
+ if (!base)
+ return;
+ prev_base = base->base;
+ }
+ }
}
static int compare_delta_entry(const void *a, const void *b)
@@ -684,13 +713,13 @@ static void parse_pack_objects(unsigned char *sha1)
progress = start_progress("Resolving deltas", nr_deltas);
for (i = 0; i < nr_objects; i++) {
struct object_entry *obj = &objects[i];
- struct base_data base_obj;
+ struct base_data *base_obj = alloc_base_data();
if (is_delta_type(obj->type))
continue;
- base_obj.obj = obj;
- base_obj.data = NULL;
- find_unresolved_deltas(&base_obj, NULL);
+ base_obj->obj = obj;
+ base_obj->data = NULL;
+ find_unresolved_deltas(base_obj);
display_progress(progress, nr_resolved_deltas);
}
}
@@ -783,20 +812,20 @@ static void fix_unresolved_deltas(struct sha1file *f, int nr_unresolved)
for (i = 0; i < n; i++) {
struct delta_entry *d = sorted_by_pos[i];
enum object_type type;
- struct base_data base_obj;
+ struct base_data *base_obj = alloc_base_data();
if (objects[d->obj_no].real_type != OBJ_REF_DELTA)
continue;
- base_obj.data = read_sha1_file(d->base.sha1, &type, &base_obj.size);
- if (!base_obj.data)
+ base_obj->data = read_sha1_file(d->base.sha1, &type, &base_obj->size);
+ if (!base_obj->data)
continue;
- if (check_sha1_signature(d->base.sha1, base_obj.data,
- base_obj.size, typename(type)))
+ if (check_sha1_signature(d->base.sha1, base_obj->data,
+ base_obj->size, typename(type)))
die("local object %s is corrupt", sha1_to_hex(d->base.sha1));
- base_obj.obj = append_obj_to_pack(f, d->base.sha1,
- base_obj.data, base_obj.size, type);
- find_unresolved_deltas(&base_obj, NULL);
+ base_obj->obj = append_obj_to_pack(f, d->base.sha1,
+ base_obj->data, base_obj->size, type);
+ find_unresolved_deltas(base_obj);
display_progress(progress, nr_resolved_deltas);
}
free(sorted_by_pos);
--
1.7.8.36.g69ee2
^ permalink raw reply related
* [PATCH v3 0/3] nd/index-pack-no-recurse
From: Nguyễn Thái Ngọc Duy @ 2012-01-14 12:19 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Shawn O. Pearce,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1326081546-29320-1-git-send-email-pclouds@gmail.com>
This round adds more explanation in commit message of 2/3 and comment
before get_base_data() in 3/3. Changes in 3/3 does not really address
Junio's concern regarding maintainability though.
It also fixes a regression in 3/3. In current code, get_base_data()
goes up as far as the first deflated parent. v2 of this series always
goes up to top parent. v3 fixes this.
Junio raised a point about depth-first vs breadth-first search in 1/3. I
have not addressed that either, but it makes me wonder if we may
benefit from using bfs in find_unresolved_deltas(), 2/3. If the delta
chains form a fork-like figure (e.g. long delta chains sharing common
base), then we may run out of cache doing dfs on one chain, by the
time we get back on the common base, we would need to deflate them
again.
Another observation is recursion in get_base_data() is unlikely to be
called in real life. With 16M default delta base cache, git.git does
not trigger it at all. Perhaps repos with large blobs have better chance..
Nguyễn Thái Ngọc Duy (3):
Eliminate recursion in setting/clearing marks in commit list
index-pack: eliminate recursion in find_unresolved_deltas
index-pack: eliminate unlimited recursion in get_base_data()
builtin/index-pack.c | 164 ++++++++++++++++++++++++++++++++++---------------
commit.c | 13 ++++-
revision.c | 45 +++++++++-----
3 files changed, 154 insertions(+), 68 deletions(-)
--
1.7.8.36.g69ee2
^ permalink raw reply
* Re: [PATCH 2/2] git-gui: fix applying line/ranges when the selection ends at the begin of a line
From: Bert Wesarg @ 2012-01-14 12:08 UTC (permalink / raw)
To: Pat Thoyts; +Cc: git, Bert Wesarg
In-Reply-To: <37339be035746797fcec7634e3560ffcd5b26cf3.1326116492.git.bert.wesarg@googlemail.com>
On Mon, Jan 9, 2012 at 14:43, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
> Selecting also the trailing newline of a line for staging/unstaging would
> have resulted in also staging/unstaging of the next line.
The fix is not complete, this logic should only be applied if we have
actually a range. I will send a replacement patch in the coming days.
Bert
>
> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
> ---
> lib/diff.tcl | 8 +++++++-
> 1 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/lib/diff.tcl b/lib/diff.tcl
> index 63f8742..a750ea7 100644
> --- a/lib/diff.tcl
> +++ b/lib/diff.tcl
> @@ -632,7 +632,13 @@ proc apply_range_or_line {x y} {
> }
>
> set first_l [$ui_diff index "$first linestart"]
> - set last_l [$ui_diff index "$last lineend"]
> + # don't include the next line if $last points to the start of a line
> + # ie. <lno>.0
> + if {[lindex [split $last .] 1] == 0} {
> + set last_l [$ui_diff index "$last -1 line lineend"]
> + } else {
> + set last_l [$ui_diff index "$last lineend"]
> + }
>
> if {$current_diff_path eq {} || $current_diff_header eq {}} return
> if {![lock_index apply_hunk]} return
> --
> 1.7.8.1.873.gfea665
>
^ permalink raw reply
* Re: [PATCH] git-gui: fix selection regression introduced in a8ca786991
From: Bert Wesarg @ 2012-01-14 11:58 UTC (permalink / raw)
To: Pat Thoyts; +Cc: git, Bert Wesarg, Shawn O. Pearce
In-Reply-To: <CAKPyHN0xRD7qYyLYaCB9u7mhCZYFObuTdJGHq-rRST-cEhTtXA@mail.gmail.com>
Hi Pat,
On Mon, Jan 9, 2012 at 10:28, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
> Hi,
>
> On Sat, Jan 7, 2012 at 20:43, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
>> While fixing the problem from a8ca786991, it introduces a regression
>> regarding what happen after the multi selected file operation (ie.
>> one of Ctrl-{T,U,J}) because the next selected file could not be handled
>> by such a subsequent file operation.
>>
>> The right way is to move the fix from this commit down into the show_diff
>> function. So that all code path add the current diff path to the list of
>> selections.
>>
>> This also simplifies helper functions for these operatione which needed
>> to handle the case whether there is only the current diff path or also
>> a selction.
>
> I think we need to think this more through, especially with input from
> Shawn, please.
>
> I have now find out, that git-gui has two selections in the file
> lists. The first is that for the current path for what we show the
> diff (the tag for this is called 'in_diff') and the the second is that
> for the current list of paths which are selected ('in_sel'). The file
> list operations 'staging', 'reverting', 'unstaging', work either on
> 'in_sel'; if that is not empty, or on 'in_diff'. The problem I've now
> realized is, that these two selections share the same visual hints,
> ie. a lightgray background.
>
> The problem I tried to solve in a8ca786991 was, that adding paths to
> the selection with Ctrl-Button-1 or Shift-cutton-1, didn't included
> the current diff path in the subsequent file list operation. But I
> would have expected it, because it was visual in the 'selection'.
>
> My current 'workaround' is to make the two selections visually
> distinguishable (and reverting a8ca786991), by using a different
> background color for the 'in_sel' tag and also the italic font, so
> that it is still possible to see whether the current diff path is in
> the selection or not:
>
> @@ -717,11 +717,11 @@ proc tk_optionMenu {w varName args} {
> proc rmsel_tag {text} {
> $text tag conf sel \
> -background [$text cget -background] \
> -foreground [$text cget -foreground] \
> -borderwidth 0
> - $text tag conf in_sel -background lightgray
> + $text tag conf in_sel -background SlateGray1 -font font_diffitalic
> bind $text <Motion> break
> return $text
> }
>
> wm withdraw .
> @@ -3557,11 +3557,11 @@ if {$use_ttk} {
> .vpane.files add .vpane.files.index -sticky news
> }
>
> foreach i [list $ui_index $ui_workdir] {
> rmsel_tag $i
> - $i tag conf in_diff -background [$i tag cget in_sel -background]
> + $i tag conf in_diff -background lightgray
> }
> unset i
>
> set files_ctxm .vpane.files.ctxm
> menu $files_ctxm -tearoff 0
>
> I'm not very pleased with this, but at least it is now possible to
> visual recognize what files will be handled by a subsequent file list
> operation.
>
> Any input is more than welcome.
>
I think we don't resolve this now, because a8ca786991 introduced a
regression introduced in 0.16, I propose to revert it for upcoming
1.7.9 release.
Thanks.
Bert
> Regards,
> Bert
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox