* Re: [PATCH] add-interactive: handle deletion of empty files
From: Junio C Hamano @ 2009-10-28 7:18 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20091028005257.GA5002@sigill.intra.peff.net>
Thanks.
^ permalink raw reply
* Re: [PATCH/RFC 2/2] completion: allow use without compiling
From: Junio C Hamano @ 2009-10-28 7:18 UTC (permalink / raw)
To: Stephen Boyd
Cc: Shawn O. Pearce, Junio C Hamano, git, Clemens Buchacher,
Sverre Rabbelier
In-Reply-To: <4AE7A1B9.5010001@gmail.com>
Stephen Boyd <bebarino@gmail.com> writes:
> Ok. Following Junio's suggestion I think we would have to do the following:
>
> (1) Revert the rename (git-completion.bash.in -> git-completion.bash)
>
> (2) Add a "generation" mode to git-completion.bash.generate to
> generate the lists and output them to a file
>
> (3) Add logic in git-completion.bash.generate to source the generated
> file if it exists
>
> (4) Source git-completion.bash.generate in git-completion.bash to get
> the functions moved there
Sorry, I do not quite see why an extra *.generate script is necessary.
^ permalink raw reply
* Re: [PATCH 5/5] http-backend: more explict LocationMatch
From: Junio C Hamano @ 2009-10-28 7:18 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Mark Lodato, git, Junio C Hamano
In-Reply-To: <20091027222435.GJ10505@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> Mark Lodato <lodatom@gmail.com> wrote:
>> In the git-http-backend examples, only match git-receive-pack within
>> /git/.
>
> All 5 patches:
>
> Acked-by: Shawn O. Pearce <spearce@spearce.org>
>
> Junio, add these to my smart-http topic please. :-)
> Thanks Mark!
Thanks.
^ permalink raw reply
* Re: [PATCH 2/3] remote-helpers: return successfully if everything up-to-date
From: Junio C Hamano @ 2009-10-28 7:18 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Clemens Buchacher, Mark Lodato, git
In-Reply-To: <20091028001125.GM10505@spearce.org>
Thanks.
^ permalink raw reply
* Re: [PATCH] Fix memory leak in transport-helper
From: Junio C Hamano @ 2009-10-28 7:18 UTC (permalink / raw)
To: Daniel Barkalow
Cc: Johannes Schindelin, Shawn O. Pearce, Junio C Hamano,
Johan Herland, Nanako Shiraishi, Sverre Rabbelier, git
In-Reply-To: <alpine.LNX.2.00.0910271456410.14365@iabervon.org>
Daniel Barkalow <barkalow@iabervon.org> writes:
> On Tue, 27 Oct 2009, Johannes Schindelin wrote:
>
>> So you mean to imply that this method is not about closing, but about
>> releasing the structure. Right?
>
> Yes, that's the word I was failing to come up with last night, thanks.
> Junio, "s/close/release/g" on that patch should improve comprehensibility
> greatly. (And changing the transport method name would probably also
> improve matters)
Thanks, both.
^ permalink raw reply
* Re: [PATCH] bash: complete more options for 'git rebase'
From: Junio C Hamano @ 2009-10-28 7:19 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Bj�rn Gustavsson, git
In-Reply-To: <20091028003208.GO10505@spearce.org>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 563 bytes --]
"Shawn O. Pearce" <spearce@spearce.org> writes:
> Bj??rn Gustavsson <bgustavsson@gmail.com> wrote:
>> Complete all long options for 'git rebase' except --no-verify
>> (probably used very seldom) and the long options corresponding
>> to -v, -q, and -f.
>>
>> Signed-off-by: Bj¶rn Gustavsson <bgustavsson@gmail.com>
>
> Acked-by: Shawn O. Pearce <spearce@spearce.org>
>
>> I am primarily interested in having the --whitespace= option completed,
>> but while at it I have added completion for the other potentially useful
>> long options.
Thanks.
^ permalink raw reply
* Re: [PATCH] Do not try to remove directories when removing old links
From: Junio C Hamano @ 2009-10-28 7:19 UTC (permalink / raw)
To: Sebastian Schuberth; +Cc: git
In-Reply-To: <hc6l9h$fn6$1@ger.gmane.org>
Thanks.
^ permalink raw reply
* Re: [PATCH] gitk: Use the --submodule option for diffs
From: Junio C Hamano @ 2009-10-28 7:19 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Paul Mackerras, Junio C Hamano, Git Mailing List
In-Reply-To: <4AE70AC9.6040302@web.de>
Jens Lehmann <Jens.Lehmann@web.de> writes:
> Instead of just showing not-quite-helpful SHA-1 pairs display the first
> lines of the corresponding commit messages in the submodule (similar to
> the output of 'git submodule summary').
>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> ---
>
> This patch applies to 'next' and uses the new --submodule option of git
> diff to achieve a more meaningful output of submodule differences in
> gitk.
>
> Any objections against making this the default?
It looks like you are not making this the default, but are making it
mandatory. That's quite different.
As long as gitk ships with matching version of git, I do not think it is a
huge problem to force "diff" to always run with --submodule option, but if
that is what the patch does, I'd prefer to see the patch says so, instead
of giving a false impression that there may be a way to disable it if one
wants to.
Looking at the patched text, I had to wonder where these $flags come from.
The callers of "diffcmd" give it to you, and I am not sure if all of them
want -p format diffs. Specifically, what does gettreediffs do? Does it
make sense to give --submodule to that invocation?
Yes, I know --submodule happens to be a no-op in non-patch format diffs,
but I do not think that is by design. It is something somebody may notice
and correct it later, at which time this patch will be broken.
I also suspect that this might break getpatchid, but as long as all the
patches consistently change/break ids it would be Ok.
^ permalink raw reply
* Re: [PATCH] gitignore: root most patterns at the top-level directory
From: Johannes Sixt @ 2009-10-28 7:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vmy3cys0f.fsf@alter.siamese.dyndns.org>
Junio C Hamano schrieb:
> How does .cvsignore and .svnignore work? Don't they have the same issue,
> and perhaps worse as I do not recall seeing a way to anchor a pattern to a
> particular directory like we do in their .SCMignore files? And judging
> from the fact that they can get away with the lack of that "feature", this
> perhaps is not an issue in real life?
.cvsignore and .svnignore do not apply recursively to subdirectories, do they?
> For example, it crossed my mind that perhaps we can change the ignore
> rules so that a non-globbing pattern is automatically anchored at the
> current directly but globbing ones are recursive as before.
>
> If we do so, there is no need to change the current .gitignore entires.
> You need to spell a concrete filename as a glob pattern that matches only
> one path if you want the recursive behaviour. E.g. if you have a Makefile
> per subdirectory, each of which generates and includes Makefile.depend
> file, you would write "Makefile.depen[d]" in the toplevel .gitignore file.
In one project that uses autotools, I have "Makefile" and "Makefile.in" in
the top-level .gitignore. I would be forced to use this ugliness instead.
Granted, to write "/git", "/git-add", etc in .gitignore is not exactly
pretty, either, but the reason that it is so extra-ugly in the git code
itself is only because there are so many build products in a single
directory that cannot be caught by a glob pattern. In practice, you
usually have only a hand-full non-glob ignored files per directory; it
doesn't hurt to anchor them using "/frotz" style.
> But that is a kind of incompatible change whose necessity is unproven and
> has to cook and wait.
I would be concerned by this change.
-- Hannes
^ permalink raw reply
* [PATCH] imap-send.c: fix pointer to be const
From: Vietor Liu @ 2009-10-28 7:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Fixes some compiler warnings:
imap-send.c: In function ‘ssl_socket_connect’:
warning: assignment discards qualifiers from pointer target type
====================================================
OpenSSL Changes between 0.9.8k and 1.0:
*) Let the TLSv1_method() etc. functions return a 'const' SSL_METHOD
pointer and make the SSL_METHOD parameter in SSL_CTX_new,
SSL_CTX_set_ssl_version and SSL_set_ssl_method 'const'.
Signed-off-by: Vietor Liu <vietor@vxwo.org>
---
imap-send.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/imap-send.c b/imap-send.c
index 3847fd1..10dd025 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -273,7 +273,7 @@ static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int ve
fprintf(stderr, "SSL requested but SSL support not compiled in\n");
return -1;
#else
- SSL_METHOD *meth;
+ const SSL_METHOD *meth;
SSL_CTX *ctx;
int ret;
--
1.6.5.2
^ permalink raw reply related
* [PATCH v2] imap-send.c: fix pointer to be const
From: Vietor Liu @ 2009-10-28 7:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Fixes some compiler warnings:
imap-send.c: In function ‘ssl_socket_connect’:
warning: assignment discards qualifiers from pointer target type
====================================================
OpenSSL Changes between 0.9.8k and 1.0:
*) Let the TLSv1_method() etc. functions return a 'const' SSL_METHOD
pointer and make the SSL_METHOD parameter in SSL_CTX_new,
SSL_CTX_set_ssl_version and SSL_set_ssl_method 'const'.
Signed-off-by: Vietor Liu <vietor@vxwo.org>
---
imap-send.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/imap-send.c b/imap-send.c
index 3847fd1..10dd025 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -273,7 +273,7 @@ static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int ve
fprintf(stderr, "SSL requested but SSL support not compiled in\n");
return -1;
#else
- SSL_METHOD *meth;
+ const SSL_METHOD *meth;
SSL_CTX *ctx;
int ret;
--
1.6.5.2
^ permalink raw reply related
* Re: [PATCH] imap-send.c: fix pointer to be const
From: Junio C Hamano @ 2009-10-28 7:25 UTC (permalink / raw)
To: Vietor Liu; +Cc: Junio C Hamano, git
In-Reply-To: <1256714677-3659-1-git-send-email-vietor@vxwo.org>
Vietor Liu <vietor@vxwo.org> writes:
> Fixes some compiler warnings:
> imap-send.c: In function ‘ssl_socket_connect’:
> warning: assignment discards qualifiers from pointer target type
>
> ====================================================
> OpenSSL Changes between 0.9.8k and 1.0:
>
> *) Let the TLSv1_method() etc. functions return a 'const' SSL_METHOD
> pointer and make the SSL_METHOD parameter in SSL_CTX_new,
> SSL_CTX_set_ssl_version and SSL_set_ssl_method 'const'.
>
> Signed-off-by: Vietor Liu <vietor@vxwo.org>
This is much easier to understand.
Thanks.
> ---
> imap-send.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/imap-send.c b/imap-send.c
> index 3847fd1..10dd025 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -273,7 +273,7 @@ static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int ve
> fprintf(stderr, "SSL requested but SSL support not compiled in\n");
> return -1;
> #else
> - SSL_METHOD *meth;
> + const SSL_METHOD *meth;
> SSL_CTX *ctx;
> int ret;
>
> --
> 1.6.5.2
^ permalink raw reply
* Re: [PATCH/RFC 2/2] completion: allow use without compiling
From: Stephen Boyd @ 2009-10-28 7:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn O. Pearce, git, Clemens Buchacher, Sverre Rabbelier
In-Reply-To: <7v63a0t29l.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Stephen Boyd <bebarino@gmail.com> writes:
>> Ok. Following Junio's suggestion I think we would have to do the following:
>>
>> (1) Revert the rename (git-completion.bash.in -> git-completion.bash)
>>
>> (2) Add a "generation" mode to git-completion.bash.generate to
>> generate the lists and output them to a file
>>
>> (3) Add logic in git-completion.bash.generate to source the generated
>> file if it exists
>>
>> (4) Source git-completion.bash.generate in git-completion.bash to get
>> the functions moved there
>
> Sorry, I do not quite see why an extra *.generate script is necessary.
I'm still assuming the generation mode has to be sh agnostic, therefore
requiring those functions to be in another *.generate script. Is that wrong?
^ permalink raw reply
* Re: [PATCH] imap-send.c: fix pointer to be const
From: Junio C Hamano @ 2009-10-28 7:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Vietor Liu, git
In-Reply-To: <7vljiwq8tc.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Vietor Liu <vietor@vxwo.org> writes:
>
>> Fixes some compiler warnings:
>> imap-send.c: In function ‘ssl_socket_connect’:
>> warning: assignment discards qualifiers from pointer target type
>>
>> ====================================================
>> OpenSSL Changes between 0.9.8k and 1.0:
>>
>> *) Let the TLSv1_method() etc. functions return a 'const' SSL_METHOD
>> pointer and make the SSL_METHOD parameter in SSL_CTX_new,
>> SSL_CTX_set_ssl_version and SSL_set_ssl_method 'const'.
>>
>> Signed-off-by: Vietor Liu <vietor@vxwo.org>
>
> This is much easier to understand.
This indeed _is_ easier to understand, but what this means is that making
"meth" const will break the compilation for people with 0.9.8k.
Their SSL_CTX_new() does not promise the callers that it won't modify the
object its parameter points at, so SSL_CTX_new(meth) will now see exactly
the same issue. You will be discarding "const" qualifier by passing it.
^ permalink raw reply
* Re: [PATCH/RFC 2/2] completion: allow use without compiling
From: Clemens Buchacher @ 2009-10-28 8:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stephen Boyd, Shawn O. Pearce, git, Sverre Rabbelier
In-Reply-To: <7v4opld631.fsf@alter.siamese.dyndns.org>
On Mon, Oct 26, 2009 at 05:38:10PM -0700, Junio C Hamano wrote:
> Clemens Buchacher <drizzd@aon.at> writes:
>
> > On Mon, Oct 26, 2009 at 04:59:18PM -0700, Junio C Hamano wrote:
> >
> >> Stephen Boyd <bebarino@gmail.com> writes:
> >>
> >> > This duplicates code, but I don't know of a way to re-use the dynamic
> >> > code without sourcing a bash script and possibly breaking someone's build.
> >>
> >> (1) If the script notices that there is a file that contains the command
> >> list, it sources it; otherwise,
> >
> > Or we substitute the command list in-place, so that we still have the entire
> > completion script in one file.
>
> That defeats the whole point of my suggestion, as you would be overwriting
> the tracked file.
Ok, not in-place then. What I meant is something like this.
git-completion.bash.in:
completion script with placeholders for command list generation code and
static command list
git-command-list.sh:
bash-agnostic command list generation script
git-completion.bash.generate:
run git-command-list.sh and replace static command list placeholder in
git-completion.bash.in, also insert command list generation code into
git-completion.bash.in, write result to git-completion.bash
Whether or not the command list should be loaded dynamically can be decided
on a per-user basis using a configuration option.
A downside of this approach is that even if we do not need the static
command list, we still need to generate the completion script. I therefore
recommend we make this part of the normal build process.
Alternatively, we could source the command list code. But it's inconvenient
to copy two completion scripts and it will probably cause confusion among
users.
Clemens
^ permalink raw reply
* [PATCH v3] imap-send.c: fix compiler warnings for OpenSSL 1.0
From: Vietor Liu @ 2009-10-28 8:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Fixes some compiler warnings:
imap-send.c: In function ‘ssl_socket_connect’:
warning: assignment discards qualifiers from pointer target type
====================================================
OpenSSL Changes between 0.9.8 and 1.0:
*) Let the TLSv1_method() etc. functions return a 'const' SSL_METHOD
pointer and make the SSL_METHOD parameter in SSL_CTX_new,
SSL_CTX_set_ssl_version and SSL_set_ssl_method 'const'.
Signed-off-by: Vietor Liu <vietor@vxwo.org>
---
imap-send.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/imap-send.c b/imap-send.c
index 3847fd1..298aa80 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -281,9 +281,9 @@ static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int ve
SSL_load_error_strings();
if (use_tls_only)
- meth = TLSv1_method();
+ meth = (SSL_METHOD *)TLSv1_method();
else
- meth = SSLv23_method();
+ meth = (SSL_METHOD *)SSLv23_method();
if (!meth) {
ssl_socket_perror("SSLv23_method");
--
1.6.5.2
^ permalink raw reply related
* Re: [PATCH] mergetool--lib: add p4merge as a pre-configured mergetool option
From: David Aguilar @ 2009-10-28 9:00 UTC (permalink / raw)
To: Charles Bailey; +Cc: Scott Chacon, git list
In-Reply-To: <20091027230043.GA11607@hashpling.org>
On Tue, Oct 27, 2009 at 11:00:43PM +0000, Charles Bailey wrote:
> On Tue, Oct 27, 2009 at 03:36:49PM -0700, Scott Chacon wrote:
> > p4merge is now a built-in diff/merge tool.
> > This adds p4merge to git-completion and updates
> > the documentation to mention p4merge.
> > ---
>
> I approve (but haven't had a chance to test this). p4merge is a
> good mergetool, but now I'll have to find something else as an example
> that you need to use custom mergetool support for.
Ditto, looks good to me.
> I'm just wondering, does this work well with unixes and Mac OS X? I
> think it's recommended install practice to symlink p4v as p4merge on
> *nix, but Mac OS X needs some sort of 'launchp4merge' to be called
> IIRC, or is this something that users can just configure with
> mergetool.p4diff.path?
I just tested this on Mac OS X with the latest version of
p4merge. It worked great.
$ git config difftool.p4merge.path \
/Applications/p4merge.app/Contents/MacOS/p4merge
$ git difftool -t p4merge HEAD^
So...
Tested-by: David Aguilar <davvid@gmail.com>
P.S. thanks for the patch, Scott.
Sorry I haven't gotten around to forking progit yet
but we did at least get Disney Animation to go with
git + github =)
http://github.com/wdas/ptex
(there's only some headers up there right now,
but we'll have more to share soon)
Have fun,
--
David
^ permalink raw reply
* [PATCH] mergetool--lib: add support for p4merge
From: Jay Soffian @ 2009-10-28 9:11 UTC (permalink / raw)
To: git; +Cc: Jay Soffian, David Aguilar, Shawn O . Pearce, Jeff King,
Junio C Hamano
Add native support for p4merge as a diff / merge tool. There are two
oddities. First, launching p4merge on Mac OS X requires running a helper
shim (which in turn is looked for in a couple common locations, as it's
very unlikely to be in PATH). Second, p4merge seems to require its file
arguments be absolute paths.
Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
As requested by an Android developer at GitTogether09.
Only tested on Mac OS X 10.6.1. Feedback from anyone running p4merge on other
OS's would be appreciated.
Documentation/git-difftool.txt | 2 +-
Documentation/git-mergetool.txt | 2 +-
git-mergetool--lib.sh | 44 ++++++++++++++++++++++++++++++++++++++-
3 files changed, 45 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt
index 96a6c51..8e9aed6 100644
--- a/Documentation/git-difftool.txt
+++ b/Documentation/git-difftool.txt
@@ -31,7 +31,7 @@ OPTIONS
Use the diff tool specified by <tool>.
Valid merge tools are:
kdiff3, kompare, tkdiff, meld, xxdiff, emerge, vimdiff, gvimdiff,
- ecmerge, diffuse, opendiff and araxis.
+ ecmerge, diffuse, opendiff, p4merge and araxis.
+
If a diff tool is not specified, 'git-difftool'
will use the configuration variable `diff.tool`. If the
diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index 68ed6c0..4a6f7f3 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -27,7 +27,7 @@ OPTIONS
Use the merge resolution program specified by <tool>.
Valid merge tools are:
kdiff3, tkdiff, meld, xxdiff, emerge, vimdiff, gvimdiff, ecmerge,
- diffuse, tortoisemerge, opendiff and araxis.
+ diffuse, tortoisemerge, opendiff, p4merge and araxis.
+
If a merge resolution program is not specified, 'git-mergetool'
will use the configuration variable `merge.tool`. If the
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index bfb01f7..23b2816 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -7,6 +7,12 @@ merge_mode() {
test "$TOOL_MODE" = merge
}
+abspath() {
+ d=$(dirname "$1")
+ d=$(cd "$d" && pwd -P)
+ echo "$d/$(basename "$1")"
+}
+
translate_merge_tool_path () {
case "$1" in
vimdiff)
@@ -21,6 +27,19 @@ translate_merge_tool_path () {
araxis)
echo compare
;;
+ p4merge)
+ # Look for the Mac OS X P4Merge shim
+ for app_dir in "/Applications" "$HOME/Applications"
+ do
+ launchp4merge="$app_dir/p4merge.app/Contents/Resources/launchp4merge"
+ if type "$launchp4merge" > /dev/null 2>&1; then
+ echo "$launchp4merge"
+ return 0
+ fi
+ done
+ # Otherwise assume we're not on Mac OS X and just return p4merge
+ echo "$1"
+ ;;
*)
echo "$1"
;;
@@ -45,7 +64,7 @@ check_unchanged () {
valid_tool () {
case "$1" in
- kdiff3 | tkdiff | xxdiff | meld | opendiff | \
+ kdiff3 | tkdiff | xxdiff | meld | opendiff | p4merge | \
emerge | vimdiff | gvimdiff | ecmerge | diffuse | araxis)
;; # happy
tortoisemerge)
@@ -284,6 +303,29 @@ run_merge_tool () {
>/dev/null 2>&1
fi
;;
+ p4merge)
+ # At least on Mac OS X p4merge wants absolute paths
+ ABS_LOCAL=$(abspath "$LOCAL")
+ ABS_REMOTE=$(abspath "$REMOTE")
+ if merge_mode; then
+ ABS_MERGED=$(abspath "$MERGED")
+ touch "$BACKUP"
+ if $base_present; then
+ ABS_BASE=$(abspath "$BASE")
+ "$merge_tool_path" \
+ "$ABS_BASE" "$ABS_LOCAL" "$ABS_REMOTE" "$ABS_MERGED" \
+ >/dev/null 2>&1
+ else
+ "$merge_tool_path" \
+ "/dev/null" "$ABS_LOCAL" "$ABS_REMOTE" "$ABS_MERGED" \
+ >/dev/null 2>&1
+ fi
+ check_unchanged
+ else
+ "$merge_tool_path" "$ABS_LOCAL" "$ABS_REMOTE" \
+ >/dev/null 2>&1
+ fi
+ ;;
*)
merge_tool_cmd="$(get_merge_tool_cmd "$1")"
if test -z "$merge_tool_cmd"; then
--
1.6.5.2.74.g610f9.dirty
^ permalink raw reply related
* Re: [PATCH] mergetool--lib: add support for p4merge
From: Jay Soffian @ 2009-10-28 9:21 UTC (permalink / raw)
To: git
Cc: Jay Soffian, David Aguilar, Shawn O . Pearce, Jeff King,
Junio C Hamano, Scott Chacon
In-Reply-To: <1256721087-72534-1-git-send-email-jaysoffian@gmail.com>
On Wed, Oct 28, 2009 at 2:11 AM, Jay Soffian <jaysoffian@gmail.com> wrote:
> Add native support for p4merge as a diff / merge tool. There are two
> oddities. First, launching p4merge on Mac OS X requires running a helper
> shim (which in turn is looked for in a couple common locations, as it's
> very unlikely to be in PATH). Second, p4merge seems to require its file
> arguments be absolute paths.
>
> Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
> ---
Amusing. I didn't see Scott's patch.
But, in my testing, for things to work properly I needed to use
launchp4merge per:
http://kb.perforce.com/AllPerforceApplications/StandAloneClients/P4merge/CommandLineP..rgeOnMacOsX
And I also found things didn't work properly unless I provided an absolute path.
(Aside, the "right" way to launch p4merge, at least on 10.6 would be:
/usr/bin/open -b com.perforce.p4merge -W -n --args <args to p4merge...>
This way OS X's launch services would find p4merge.app wherever it is
on the user's system. But, I think some of these options to open are
10.6 specific and in practice looking in /Applications and
$HOME/Applications I think is a sane enough default.)
j.
^ permalink raw reply
* Re: [PATCH] mergetool--lib: add support for p4merge
From: David Aguilar @ 2009-10-28 9:27 UTC (permalink / raw)
To: Jay Soffian
Cc: git, Shawn O . Pearce, Jeff King, Junio C Hamano, Scott Chacon
In-Reply-To: <76718490910280221u4e1d3e78me7f9b0b45f590e56@mail.gmail.com>
On Wed, Oct 28, 2009 at 02:21:46AM -0700, Jay Soffian wrote:
> On Wed, Oct 28, 2009 at 2:11 AM, Jay Soffian <jaysoffian@gmail.com> wrote:
> > Add native support for p4merge as a diff / merge tool. There are two
> > oddities. First, launching p4merge on Mac OS X requires running a helper
> > shim (which in turn is looked for in a couple common locations, as it's
> > very unlikely to be in PATH). Second, p4merge seems to require its file
> > arguments be absolute paths.
> >
> > Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
> > ---
>
> Amusing. I didn't see Scott's patch.
Apologies Jay!
It's late and I saw Scott listed in the CC: and guessed wrong ;)
Thanks for the patch again, man. Good stuff.
>
> But, in my testing, for things to work properly I needed to use
> launchp4merge per:
>
> http://kb.perforce.com/AllPerforceApplications/StandAloneClients/P4merge/CommandLineP..rgeOnMacOsX
>
> And I also found things didn't work properly unless I provided an absolute path.
>
> (Aside, the "right" way to launch p4merge, at least on 10.6 would be:
>
> /usr/bin/open -b com.perforce.p4merge -W -n --args <args to p4merge...>
>
> This way OS X's launch services would find p4merge.app wherever it is
> on the user's system. But, I think some of these options to open are
> 10.6 specific and in practice looking in /Applications and
> $HOME/Applications I think is a sane enough default.)
>
> j.
--
David
^ permalink raw reply
* [PATCH] help -a: do not unnecessarily look for a repository
From: Gerrit Pape @ 2009-10-28 9:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7viqe0yrnu.fsf@alter.siamese.dyndns.org>
Although 'git help -a' actually doesn't need to be run inside a git
repository and uses no repository-specific information, it looks for a
git directory. On 'git <TAB><TAB>' the bash completion runs 'git help
-a' and unnecessary searching for a git directory can be annoying in
auto-mount environments. With this commit, 'git help' no longer
searches for a repository when run with the -a option.
The fix is from Johannes Schindelin, the annoying behavior has been
reported by Vincent Danjean through
http://bugs.debian.org/539273
Signed-off-by: Gerrit Pape <pape@smarden.org>
---
On Tue, Oct 27, 2009 at 11:11:01PM -0700, Junio C Hamano wrote:
> Gerrit Pape <pape@smarden.org> writes:
> > Hi Junio, I suggest to apply this patch from Johannes to master.
> Could you help by coming up with a suitable log message?
>
> It's a bit too much to ask me to hunt for ancient discussion to
> correct the <<all the ack go here>> myself to describe what the
> issue was,
Sure, sorry for that. Regards, Gerrit.
builtin-help.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/builtin-help.c b/builtin-help.c
index e1ade8e..ca08519 100644
--- a/builtin-help.c
+++ b/builtin-help.c
@@ -417,9 +417,6 @@ int cmd_help(int argc, const char **argv, const char *prefix)
const char *alias;
load_command_list("git-", &main_cmds, &other_cmds);
- setup_git_directory_gently(&nongit);
- git_config(git_help_config, NULL);
-
argc = parse_options(argc, argv, prefix, builtin_help_options,
builtin_help_usage, 0);
@@ -430,6 +427,9 @@ int cmd_help(int argc, const char **argv, const char *prefix)
return 0;
}
+ setup_git_directory_gently(&nongit);
+ git_config(git_help_config, NULL);
+
if (!argv[0]) {
printf("usage: %s\n\n", git_usage_string);
list_common_cmds_help();
--
1.6.5.2
^ permalink raw reply related
* Re: [PATCH] mergetool--lib: add support for p4merge
From: David Aguilar @ 2009-10-28 9:36 UTC (permalink / raw)
To: Jay Soffian
Cc: git, Shawn O . Pearce, Jeff King, Junio C Hamano, Scott Chacon
In-Reply-To: <76718490910280221u4e1d3e78me7f9b0b45f590e56@mail.gmail.com>
On Wed, Oct 28, 2009 at 02:21:46AM -0700, Jay Soffian wrote:
>
> But, in my testing, for things to work properly I needed to use
> launchp4merge per:
>
> http://kb.perforce.com/AllPerforceApplications/StandAloneClients/P4merge/CommandLineP..rgeOnMacOsX
>
> And I also found things didn't work properly unless I provided an absolute path.
>
> (Aside, the "right" way to launch p4merge, at least on 10.6 would be:
>
> /usr/bin/open -b com.perforce.p4merge -W -n --args <args to p4merge...>
:(
I tested on 10.5, so there's definitely some difference in
behavior since difftool.p4merge.path is all that was needed here
(with an absolute path as I mentioned).
> This way OS X's launch services would find p4merge.app wherever it is
> on the user's system. But, I think some of these options to open are
> 10.6 specific and in practice looking in /Applications and
> $HOME/Applications I think is a sane enough default.)
We've stayed away from hard-coding any platform-specific paths
in mergetool--lib in the past. It's a practicality thing --
trying to guess all of the possible installation locations is
simply untenable.
Here's an old thread where we talked about this in the context
of ecmerge:
http://thread.gmane.org/gmane.comp.version-control.git/118125/focus=118182
Let me know what you think.
--
David
^ permalink raw reply
* [PATCH] bash completion: difftool accepts the same options as diff
From: Markus Heidelberg @ 2009-10-28 9:45 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git, David Aguilar, Markus Heidelberg
So complete refs, files after the doubledash and some diff options that
make sense for difftool.
Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---
contrib/completion/git-completion.bash | 10 ++++++++--
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index d3fec32..45369e2 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -958,6 +958,8 @@ __git_mergetools_common="diffuse ecmerge emerge kdiff3 meld opendiff
_git_difftool ()
{
+ __git_has_doubledash && return
+
local cur="${COMP_WORDS[COMP_CWORD]}"
case "$cur" in
--tool=*)
@@ -965,11 +967,15 @@ _git_difftool ()
return
;;
--*)
- __gitcomp "--tool="
+ __gitcomp "--cached --staged --pickaxe-all --pickaxe-regex
+ --base --ours --theirs
+ --no-renames --diff-filter= --find-copies-harder
+ --relative --ignore-submodules
+ --tool="
return
;;
esac
- COMPREPLY=()
+ __git_complete_file
}
__git_fetch_options="
--
1.6.5.2.86.g61663
^ permalink raw reply related
* Re: [PATCH] mergetool--lib: add support for p4merge
From: Jay Soffian @ 2009-10-28 9:52 UTC (permalink / raw)
To: David Aguilar
Cc: git, Shawn O . Pearce, Jeff King, Junio C Hamano, Scott Chacon
In-Reply-To: <20091028093655.GC90780@gmail.com>
On Wed, Oct 28, 2009 at 2:36 AM, David Aguilar <davvid@gmail.com> wrote:
> I tested on 10.5, so there's definitely some difference in
> behavior since difftool.p4merge.path is all that was needed here
> (with an absolute path as I mentioned).
There are two issues here:
1) Is it necessary to use launchp4merge, or is calling p4merge directly okay:
Calling p4merge directly works, but the advantage of calling
launchp4merge instead is that it works a little more elegantly:
- If p4merge is already running, using launchp4merge opens a new
window inside the running instance, instead of launching p4merge
anew.
- After performing the merge, it is sufficient to just close the merge
window, as opposed to having to quit the p4merge application (which
you have to do if you run p4merge directly).
2) Does p4merge/launchp4merge require absolute paths for its arguments.
I found that it does. So I wonder whether possibly you tested with
difftool, which uses absolute arguments, as opposed to mergetool,
which uses relative arguments. You'd only see the issue with
mergetool.
> We've stayed away from hard-coding any platform-specific paths
> in mergetool--lib in the past. It's a practicality thing --
> trying to guess all of the possible installation locations is
> simply untenable.
>
> Here's an old thread where we talked about this in the context
> of ecmerge:
>
> http://thread.gmane.org/gmane.comp.version-control.git/118125/focus=118182
>
> Let me know what you think.
Disagree with the conclusion of that thread. On Linux, PATH is very
likely to have the merge/diff binary in it. On OS X, this is highly
unlikely, but there are two very common/likely locations we can look,
/Applications and $HOME/Applications.
j.
^ permalink raw reply
* Re: [PATCH] mergetool--lib: add support for p4merge
From: Jay Soffian @ 2009-10-28 10:02 UTC (permalink / raw)
To: David Aguilar
Cc: git, Shawn O . Pearce, Jeff King, Junio C Hamano, Scott Chacon
In-Reply-To: <76718490910280252n7a2c41baj5e220c784f1f3617@mail.gmail.com>
On Wed, Oct 28, 2009 at 2:52 AM, Jay Soffian <jaysoffian@gmail.com> wrote:
> 2) Does p4merge/launchp4merge require absolute paths for its arguments.
>
> I found that it does.
I lie. Running p4merge directly works with relative paths. Launching
p4merge via launchp4merge (or via open) requires absolute paths, which
I guess makes sense.
I'd still lean toward using launchp4merge with absolute paths.
$0.02.
j.
^ 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