* Re: [PATCH v4] imap-send.c: fix compiler warnings for OpenSSL 1.0
From: Junio C Hamano @ 2009-10-31 21:34 UTC (permalink / raw)
To: Vietor Liu; +Cc: Junio C Hamano, git
In-Reply-To: <1256970963-6345-1-git-send-email-vietor@vxwo.org>
Thanks.
^ permalink raw reply
* Re: [PATCH] commit -c/-C/--amend: take over authorship and restamp time with --claim
From: Junio C Hamano @ 2009-10-31 21:24 UTC (permalink / raw)
To: Erick Mattos; +Cc: git
In-Reply-To: <1256958480-6775-1-git-send-email-erick.mattos@gmail.com>
Erick Mattos <erick.mattos@gmail.com> writes:
> The new --claim option is meant to solve this need by regenerating the
> timestamp and setting as new author the committer or the one specified
> on --author option.
I'll leave discussion on the option name to others.
> +--claim::
> + When used with -C/-c/--amend options the committer takes over
> + the cloned commit authorship and renew the timestamp thus using
> + only the commit message from the source.
"The cloned commit" is a bit misleading; in the mind of users --amend does
not clone but rewrite, as the old commit usually only belongs to a reflog
and not any other branch. I'd rewrite it this way, perhaps.
When used with -C/-c/--amend options, declare that the authorship
of the resulting commit now belongs of the committer. This also
renews the author timestamp.
We also would need a test to protect this new feature from getting broken
by future updates.
Thanks.
^ permalink raw reply
* Re: [PATCH 7/8] Provide a build time default-editor setting
From: Jonathan Nieder @ 2009-10-31 21:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
In-Reply-To: <7vzl775ol5.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Honestly speaking, my preference is to see if the built-in editor is
> exactly spelled as 'v' 'i', and skip this test altogether if it isn't.
That does make sense. But let me try one last time, before doing
that. (I should have sat down and thought this through carefully
before sending the first version --- sorry.)
Though the first two iterations of the patch were pretty ugly, the
third was just 's/vi/"$editor"/g' after setting editor and bailing out
if it does not consist of lowercase letters. As you mentioned, it
makes more sense to skip only the "vi" part of the test.
Tested with DEFAULT_EDITOR=vi, vim, /usr/bin/nonexistent.
t/t7005-editor.sh | 37 +++++++++++++++++++++++++------------
1 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/t/t7005-editor.sh b/t/t7005-editor.sh
index a95fe19..5257f4d 100755
--- a/t/t7005-editor.sh
+++ b/t/t7005-editor.sh
@@ -4,7 +4,21 @@ test_description='GIT_EDITOR, core.editor, and stuff'
. ./test-lib.sh
-for i in GIT_EDITOR core_editor EDITOR VISUAL vi
+unset EDITOR VISUAL GIT_EDITOR
+
+test_expect_success 'determine default editor' '
+
+ vi=$(TERM=vt100 git var GIT_EDITOR) &&
+ test -n "$vi"
+
+'
+
+if ! expr "$vi" : '^[a-z]*$' >/dev/null
+then
+ vi=
+fi
+
+for i in GIT_EDITOR core_editor EDITOR VISUAL $vi
do
cat >e-$i.sh <<-EOF
#!$SHELL_PATH
@@ -12,19 +26,18 @@ do
EOF
chmod +x e-$i.sh
done
-unset vi
-mv e-vi.sh vi
-unset EDITOR VISUAL GIT_EDITOR
+
+if ! test -z "$vi"
+then
+ mv e-$vi.sh $vi
+fi
test_expect_success setup '
- msg="Hand edited" &&
+ msg="Hand-edited" &&
+ test_commit "$msg" &&
echo "$msg" >expect &&
- git add vi &&
- test_tick &&
- git commit -m "$msg" &&
- git show -s --pretty=oneline |
- sed -e "s/^[0-9a-f]* //" >actual &&
+ git show -s --format=%s > actual &&
diff actual expect
'
@@ -54,7 +67,7 @@ test_expect_success 'dumb should prefer EDITOR to VISUAL' '
TERM=vt100
export TERM
-for i in vi EDITOR VISUAL core_editor GIT_EDITOR
+for i in $vi EDITOR VISUAL core_editor GIT_EDITOR
do
echo "Edited by $i" >expect
unset EDITOR VISUAL GIT_EDITOR
@@ -78,7 +91,7 @@ done
unset EDITOR VISUAL GIT_EDITOR
git config --unset-all core.editor
-for i in vi EDITOR VISUAL core_editor GIT_EDITOR
+for i in $vi EDITOR VISUAL core_editor GIT_EDITOR
do
echo "Edited by $i" >expect
case "$i" in
^ permalink raw reply related
* Re: Add '--bisect' revision machinery argument
From: Christian Couder @ 2009-10-31 20:58 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.2.01.0910281631400.31845@localhost.localdomain>
On Thursday 29 October 2009, Linus Torvalds wrote:
> On Wed, 28 Oct 2009, Junio C Hamano wrote:
> > This shows a very nice direction to evolve, but your patch as-is breaks
> > "rev-list --bisect", I think.
>
> I think you're right. I tested git rev-parse, and the 'git log'
> machinery, but I didn't think about the fact that we already had a
> meaning
> for '--bisect' in rev-list.
>
> > Also, the helper of "git bisect" can and probably should be taught to
> > just ask this new behaviour from the revision machinery, instead of
> > collecting good and bad refs itself using bisect.c::read_bisect_refs().
>
> Yeah. And git-bisect.sh can be simplified too.
I will have a look at that.
Sorry for not responding earlier but I just came back today from the Linux
Kongress 2009 in Dresden (http://www.linux-kongress.org/2009/).
Best regards,
Christian.
^ permalink raw reply
* [PATCH 2/2] configure: allow user to set gitconfig, pager and editor
From: Ben Walton @ 2009-10-31 20:41 UTC (permalink / raw)
To: gitster, jrnieder, git; +Cc: Ben Walton
In-Reply-To: <1257021695-21260-2-git-send-email-bwalton@artsci.utoronto.ca>
Use the new GIT_WITH_MAKE_VAR function to allow user specified
values for ETC_GITCONFIG, DEFAULT_PAGER and DEFAULT_EDITOR.
Signed-off-by: Ben Walton <bwalton@artsci.utoronto.ca>
---
configure.ac | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/configure.ac b/configure.ac
index 2829dbb..d50d492 100644
--- a/configure.ac
+++ b/configure.ac
@@ -241,6 +241,16 @@ GIT_PARSE_WITH(iconv))
# change being considered an inode change from the update-index perspective.
#
+# Allow user to set ETC_GITCONFIG variable
+GIT_WITH_MAKE_VAR(gitconfig, ETC_GITCONFIG)
+#
+# Allow user to set the default pager
+GIT_WITH_MAKE_VAR(pager, DEFAULT_PAGER)
+#
+# Allow user to set the default editor
+GIT_WITH_MAKE_VAR(editor, DEFAULT_EDITOR)
+
+#
# Define SHELL_PATH to provide path to shell.
GIT_ARG_SET_PATH(shell)
#
--
1.6.5
^ permalink raw reply related
* [PATCH 0/2] Set Makefile variables from configure
From: Ben Walton @ 2009-10-31 20:41 UTC (permalink / raw)
To: gitster, jrnieder, git; +Cc: Ben Walton
These patches add support for setting the newly created DEFAULT_EDITOR
and DEFAULT_PAGER from the configure script. I also tacked in
ETC_GITCONFIG, since I can't currently toggle this without setting a
command line value when building, but have need to alter it.
The function added is generic, and will allow for setting new
variables as needed in the future. No validation is done on the
values. It is less specific than the *_PATH setting functions that
already exist.
Ben Walton (2):
configure: add function to directly set Makefile variables
configure: allow user to set gitconfig, pager and editor
configure.ac | 24 ++++++++++++++++++++++++
1 files changed, 24 insertions(+), 0 deletions(-)
^ permalink raw reply
* [PATCH 1/2] configure: add function to directly set Makefile variables
From: Ben Walton @ 2009-10-31 20:41 UTC (permalink / raw)
To: gitster, jrnieder, git; +Cc: Ben Walton
In-Reply-To: <1257021695-21260-1-git-send-email-bwalton@artsci.utoronto.ca>
Add function GIT_WITH_MAKE_VAR to provide an easy way to allow user
input to directly specify values for variables in the Makefile.
An example use is:
GIT_WITH_MAKE_VAR(gitconfig, ETC_GITCONFIG)
This would allow the user to add --with-gitconfig=/etc/mysiteconf
to their ./configure command line to add
ETC_GITCONFIG=/etc/mysiteconf to the config.mak.autogen file.
Signed-off-by: Ben Walton <bwalton@artsci.utoronto.ca>
---
configure.ac | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/configure.ac b/configure.ac
index 84b6cf4..2829dbb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -68,6 +68,20 @@ else \
GIT_CONF_APPEND_LINE(${PACKAGE}DIR=$withval); \
fi \
])# GIT_PARSE_WITH
+dnl
+dnl GIT_WITH_MAKE_VAR(withname, VAR)
+dnl ---------------------
+dnl Set VAR to the value specied by --with-$withname if --with-$withname
+dnl is specified. This is a direct way to allow setting variables in the
+dnl Makefile.
+AC_DEFUN([GIT_WITH_MAKE_VAR],
+[AC_ARG_WITH([$1],
+ [AS_HELP_STRING([--with-$1=VALUE],
+ [provide value for $2])],
+ if test -n "$withval"; then \
+ AC_MSG_NOTICE([Setting $2 to $withval]); \
+ GIT_CONF_APPEND_LINE($2=$withval); \
+ fi)])# GIT_WITH_MAKE_VAR
dnl
dnl GIT_CHECK_FUNC(FUNCTION, IFTRUE, IFFALSE)
--
1.6.5
^ permalink raw reply related
* Re: [PATCH] Update packfile transfer protocol documentation
From: Johannes Sixt @ 2009-10-31 20:12 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Scott Chacon, git list
In-Reply-To: <20091031020634.GL10505@spearce.org>
Shawn O. Pearce schrieb:
> Scott Chacon <schacon@gmail.com> wrote:
>> +Currently only 'host' is allowed in the extra information. It's
>
> No. We should make this a MUST. As in:
>
> Only host-parameter is allowed in the git-proto-request.
> Clients MUST NOT attempt to send additional parameters.
>
> Sending another header can cause older git-daemons to lock up.
I think you are making a wrong case here: Older git-daemons that lock up
are security holes because they allow DoS attacks, and any decent admin
will want to upgrade to a fixed git-daemon anyway.
Fixed git-daemons can allow extra information in addition to 'host'. I
know you argued otherwise when you submitted the fix to git-daemon, but I
think you were wrong already back then.
-- Hannes
^ permalink raw reply
* Re: [PATCH 7/8] Provide a build time default-editor setting
From: Junio C Hamano @ 2009-10-31 19:51 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
In-Reply-To: <20091031032647.GA5583@progeny.tock>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Junio C Hamano wrote:
>> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>>> +test_expect_success 'does editor have a simple name (no slashes, etc)?' '
>>> +
>>> + editor=$(TERM=vt100 git var GIT_EDITOR) &&
>>> + test -n "$editor" &&
>>> + simple=t &&
>>> + case "$editor" in
>>> + */* | core_editor | [A-Z]*)
>>
>> Hmm, what are the latter two cases designed to catch?
>
> Both are meant to allow the test to work without too many changes.
Honestly speaking, my preference is to see if the built-in editor is
exactly spelled as 'v' 'i', and skip this test altogether if it isn't.
Then the patch only needs to insert these lines (and reword "default
editor name too complicated" to "using customized default editor") without
touching the rest. It simply does not look worth the complication.
You _might_ be able to skip only the "vi" part of the test when you see
that the built-in default is customized, though. I didn't look closely
enough.
> diff --git a/t/t7005-editor.sh b/t/t7005-editor.sh
> ...
> +unset EDITOR VISUAL GIT_EDITOR
> +
> +test_expect_success 'determine default editor' '
> +
> + editor=$(TERM=vt100 git var GIT_EDITOR) &&
> + test -n "$editor"
> +
> +'
> +
> +if ! test -z "$(printf '%s\n' "$editor" | sed '/^[a-z]*$/d')"
> +then
> + say 'skipping editor tests, default editor name too complicated'
> + test_done
> +fi
> +
^ permalink raw reply
* Re: [PATCH v2 3/8] Teach git var about GIT_EDITOR
From: Johannes Sixt @ 2009-10-31 19:40 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Ben Walton, David Roundy, GIT List
In-Reply-To: <20091031013934.GD5160@progeny.tock>
Jonathan Nieder schrieb:
> From: Johannes Sixt <j6t@kdbg.org>
>
> Expose the command used by launch_editor() for scripts to use...
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
This patch has grown far beyond my original submission. I can't validly
sign it off anymore. Please take authorship ;)
-- Hannes
^ permalink raw reply
* Re: [PATCH] Don't create the $GIT_DIR/branches directory on init
From: Junio C Hamano @ 2009-10-31 19:32 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Thomas Rast, Robin Rosenberg, git, sasa.zivkov
In-Reply-To: <20091031182416.GO10505@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
>> This patch alone breaks tests in the t55?? series quite a lot,
>
> Drop the patch.
Ok, I missed that the unstated goal was "we will eventually drop support
for reading from branches and remotes". I think it is a worthy goal, but
also agree that should be done at 1.7.0 or 1.8.0 boundary.
If this were Andrew alone, I personally do not think it is such a big
deal. My understanding is that an eventual goal over there in the kernel
land is to grow 'linux-next' tree even more so that akpm tree will shrink
in the longer term, anyway. I consulted Andrew in early days while he was
fighting with git to get his scripts to do what he needs them to do, and I
can do that again to bring them up-to-date if necessary.
In order to better support "massive integrator" workflow
that involves interacting with dozens of remote branches, we need to admit
that some of the things you can do if you are set up like Andrew are less
convenient to do via "git remotes" managing .git/config file. E.g.
: add new
$ echo "$korg:/home/rmk/linux-2.6-arm.git#master" >.git/branches/arm-current
: remove stale
$ rm -f .git/branches/powerpc
: find source
$ grep -r $something .git/branches
: make random small changes, e.g. change branch name only
$ vi .git/branches/sparc
and we _might_ need to improve "git remote" interface before dropping
support for reading branches and remotes files.
Admittedly, managing integration trees like -mm and linux-next needs not
just nickname-to-repo-branch mapping but also involves the correct merge
order anyway, and people like Andrew and Stephan Rothwell (linux-next)
maintain a text file to describe them that git does not know about.
E.g. http://linux.f-seidel.de/linux-next/pmwiki/pmwiki.php?n=Linux-next.IncludedTrees
We do not have any infrastructure support for that kind of thing.
To manage 'pu' and 'next', I use specialized scripts (in my 'todo' branch,
look for Reintegrate script) myself, even though the number of topics I
manage is far smaller than what we are discussing here[*1*]. In that
sense, the difference between the remotes sections in .git/config file and
.git/branches/* files is not such a big issue in the larger picture.
As long as we keep the UI to deal with bare URL and branch names from the
command line properly working, the "massive intergrator" workflow might be
better done without _any_ remote definition, either in the config nor
branches/remotes files. Two integrator scripts might read like these:
#!/bin/sh
# fetch-all script
# fetch from repositories
failed=
while read nickname url branch
do
git fetch -q "$url" "+$branch:refs/remotes/$nickname" ||
failed="$failed$nickname "
done <merge-order
test -z "$failed" ||
echo "Failed to fetch from $failed"
#!/bin/sh
# merge-all script
# git reset --hard remotes/linus-tip
while read nickname url branch
do
git merge -m "Merge from $url#$branch" "remotes/$nickname" ||
accept_rerere ||
break
done <merge-order
where accept_rerere is something like what my Reintegrate script (in
'todo' branch) has in it. Then the workflow for the integrator would
become:
1. to run "fetch-all" once;
2. reset to Linus's tip of the day;
3. run "merge-all";
3.a fix up conflicts;
edit && git commit
3.b decide to drop the day's tree and use previous day's:
git reset --hard &&
git update-ref refs/remotes/$nick refs/remotes/$nick@{1.day}
3.c decide to drop the tree:
git reset --hard &&
edit merge-order
and go back to step 3.
[Footnote]
*1* I do not keep a "merge order" file, but existing merges on 'pu' for
that purpose. The Reintegrate script figures it out by looking at what
was in 'pu'. One cycle of my git day looks like this, in this order:
: record what topics are in 'next' and 'pu'
: 'jch' is a shadow of 'next' that merges all the topics in 'next'
: on top of 'master'.
$ Meta/Reintegrate master..jch >/var/tmp/redo-jch.sh
$ Meta/Reintegrate jch..pu >/var/tmp/redo-pu.sh
: queue a new topic
$ git checkout -b xx/topic master
$ git am -s $patch
: update a topic
$ git checkout xx/topic
$ git am -s $patch
: fix a topic (that is not in 'next' yet)
$ git checkout xx/topic
$ git rebase -i $(git merge-base master HEAD)
: decide to graduate a topic to 'master'
$ git checkout master
$ git merge xx/topic
: apply directly to master
$ git checkout master
$ git am -s $patch
: update 'next' with what's new in 'master'
$ git checkout next && git merge master
: rebuild 'jch' (shadow of 'next')
$ git branch -f jch master && git checkout jch
$ sh /var/tmp/redo-jch.sh
: at this point, 'jch' and 'next' must exactly match
: add topics that are next-ready to 'jch' and test
$ git merge xx/topic
: merge them to 'next' as well
$ Meta/Reintegrate master..jch >/var/tmp/redo-jch.sh
$ git checkout next && sh /var/tmp/redo-jch.sh
: at this point, 'jch' and 'next' must exactly match
: rebuild 'pu'
$ git branch -f pu jch && git checkout pu
$ sh /var/tmp/redo-pu.sh
: merge new topics
$ git merge xx/topic
^ permalink raw reply
* Re: [PATCH] Don't create the $GIT_DIR/branches directory on init
From: Shawn O. Pearce @ 2009-10-31 18:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Thomas Rast, Robin Rosenberg, git, sasa.zivkov
In-Reply-To: <7vr5sj8m5f.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> Modern git Porcelains write remote definitions solely to .git/config, but
> still reads from .git/{branches,remotes}.
...
> Andrew Morton explicitly asked for this to be kept a few years
> ago and I do not see a reason to deprecate this.
...
> Shawn and other wants to stop JGit from creating this directory on
I probably said something like this. I won't bother denying it,
because list archives are more accurate than my own fallible memory.
But I didn't know the Andrew Morton part above. After hearing it
from you, I'm reversing my (apparent) direction here. We should
continue to create the branches directory within a new repository.
Sorry Robin, but Andrew Morton matters. Its one stupid unused
directory in a repository that will chew through thousands of inodes
as loose objects. Its a drop in the bucket in terms of resource
cost used by Git. And Andrew is someone whose workflow we don't
want to break if we can avoid it. He's a long time Git user who is
also high up in the kernel food chain. Interrupting him disrupts
a fair chunk of kernel work while he grumbles about the Goddamn
Idiotic Truckload of s**t that Linus begat.
> This patch alone breaks tests in the t55?? series quite a lot,
Drop the patch.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] Don't create the $GIT_DIR/branches directory on init
From: Junio C Hamano @ 2009-10-31 18:15 UTC (permalink / raw)
To: Thomas Rast; +Cc: Robin Rosenberg, Junio C Hamano, git, spearce, sasa.zivkov
In-Reply-To: <200910311011.31189.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> Robin Rosenberg wrote:
>> Git itself does not even look at this directory.
Modern git Porcelains write remote definitions solely to .git/config, but
still reads from .git/{branches,remotes}. What we do not do is to update
these locations, and we do not need to have these locations to operate.
So "not even look at" is too strong; it just "not touch".
I do not think there is reason to change that part of the equation. For
people who need to fetch and merge hundreds of random places, it is a lot
handier to be able to do
echo "$url#$branch" >.git/branch/$nickname
rm .git/branch/$nickname
to manage the set of locations added to and deleted from the daily
compose. Andrew Morton explicitly asked for this to be kept a few years
ago and I do not see a reason to deprecate this.
Now, not installing an empty .git/branch directory does break the above
workflow. You would need to mkdir _once_ yourself, but I do not think
that is such a big deal.
On the other hand, I do not think it is such a big deal to have otherwise
unused .git/branches/ directory, either. Robin wrote:
Shawn and other wants to stop JGit from creating this directory on
init with the motivation that newer Git version doesn't create it
anymore. This patch would make that assertion true.
and after re-reading it, I realize "the motivation" is not a motivation at
all---it is merely an excuse ("after this patch is applied, git wouldn't
create it anymore"---so JGit will have an excuse not to do so). It does
not say _why_ it shouldn't be there in the first place. IOW, we need to
fill in the blank in: "JGit is merely following suit; the reason git
stopped creating the directory is ________").
This patch alone breaks tests in the t55?? series quite a lot, and I am
tempted to revert it. My time is more valuable than fixing the fallouts
from this change, when the real purpose of the change is not yet stated.
^ permalink raw reply
* Re: [PATCH] Don't create the $GIT_DIR/branches directory on init
From: Shawn O. Pearce @ 2009-10-31 18:09 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: Thomas Rast, Junio C Hamano, git, sasa.zivkov
In-Reply-To: <200910311902.48317.robin.rosenberg@dewire.com>
Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> > * a remote in the git configuration file: `$GIT_DIR/config`,
> > * a file in the `$GIT_DIR/remotes` directory, or
> > * a file in the `$GIT_DIR/branches` directory.
>
> I, and a few other people, it seems. Seems the purpose of these
> files is a bit different. Git does look in these directories (both)
> when fetch is run. Seems remotes is not created by init though.
Since remotes isn't created by init, branches shouldn't be either.
Cogito is dead, and that was the main customer who wanted branches
to be present in a repository.
I think its safe to remove branches from the template repository
and stop creating it, but continue to read from branches and
remotes if they exist.
We might want to consider dropping support for them in 1.7.0
or 1.8.0, because any new tools largely focus on config.
E.g. git-remote probably can't edit branches or remotes, git-gui
probably doesn't use them, JGit doesn't use them.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] Don't create the $GIT_DIR/branches directory on init
From: Robin Rosenberg @ 2009-10-31 18:02 UTC (permalink / raw)
To: Thomas Rast; +Cc: Junio C Hamano, git, spearce, sasa.zivkov
In-Reply-To: <200910311011.31189.trast@student.ethz.ch>
lördag 31 oktober 2009 10:11:29 skrev Thomas Rast:
> Robin Rosenberg wrote:
> > Git itself does not even look at this directory.
>
> This contradicts the git-fetch manpage though: from urls-remotes.txt,
> it includes
>
> The name of one of the following can be used instead
> of a URL as `<repository>` argument:
>
> * a remote in the git configuration file: `$GIT_DIR/config`,
> * a file in the `$GIT_DIR/remotes` directory, or
> * a file in the `$GIT_DIR/branches` directory.
>
> (and a longer explanation of what they need to look like).
>
> So which one is wrong?
I, and a few other people, it seems. Seems the purpose of these
files is a bit different. Git does look in these directories (both)
when fetch is run. Seems remotes is not created by init though.
-- robin
^ permalink raw reply
* Re: [PATCH 10/19] Allow helpers to request marks for fast-import
From: Sverre Rabbelier @ 2009-10-31 16:19 UTC (permalink / raw)
To: Johan Herland; +Cc: git, Johannes Schindelin, Daniel Barkalow
In-Reply-To: <200910311304.05408.johan@herland.net>
Heya,
On Sat, Oct 31, 2009 at 13:04, Johan Herland <johan@herland.net> wrote:
> On Friday 30 October 2009, Sverre Rabbelier wrote:
> This conglomeration of patch series is becoming fairly complicated, and it's
> becoming hard to stay in sync. I suggest that you drop the CVS-specific
> parts from this series, and work on stabilizing the common infrastructure.
> Once that has settled, you can send a git-remote-hg series, and I can send a
> rebased and updated git-remote-cvs series.
That sounds like a good idea; I want to use the marks in git-remote-hg as well.
> Feel free to reorganize the patches so that the git_remote_helpers
> infrastructure is created in the correct location (instead of reorganizing
> git_remote_cvs).
Ok, will do.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* [PATCH nd/sparse] Support directory exclusion from index
From: Nguyễn Thái Ngọc Duy @ 2009-10-31 14:09 UTC (permalink / raw)
To: git, Junio C Hamano, skillzero; +Cc: Nguyễn Thái Ngọc Duy
The function excluded_from_list (or its public API excluded) is
currently used to mark what entry is included in sparse checkout.
Because index does not have directories, the pattern "foo", while
would match directory "foo" on working directory, would not match
against index.
To overcome this, if a pattern does not match, check if it matches
parent directories too before moving on to the next pattern. This
behavior only applies to index matching.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
There is subtle difference, which might break things. When working
with file system, it checks top-down, parent directories first.
I do bottom-up.
If/you/have/deep/directories sparse checkout may become slow.
And my test broke :(
dir.c | 24 ++++++++++++++++++++++--
t/t1009-read-tree-sparse-checkout.sh | 4 ++--
2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/dir.c b/dir.c
index 3a8d3e6..82227e5 100644
--- a/dir.c
+++ b/dir.c
@@ -334,13 +334,15 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
}
/* Scan the list and let the last match determine the fate.
+ * dtype == NULL means matching against index, not working directory.
* Return 1 for exclude, 0 for include and -1 for undecided.
*/
-int excluded_from_list(const char *pathname,
- int pathlen, const char *basename, int *dtype,
+int excluded_from_list(const char *pathname_,
+ int pathlen_, const char *basename_, int *dtype,
struct exclude_list *el)
{
int i;
+ char buf[PATH_MAX];
if (el->nr) {
for (i = el->nr - 1; 0 <= i; i--) {
@@ -348,6 +350,11 @@ int excluded_from_list(const char *pathname,
const char *exclude = x->pattern;
int to_exclude = x->to_exclude;
+ const char *pathname = pathname_;
+ const char *basename = basename_;
+ int pathlen = pathlen_;
+
+recheck:
if (x->flags & EXC_FLAG_MUSTBEDIR) {
if (!dtype) {
if (!prefixcmp(pathname, exclude))
@@ -398,6 +405,19 @@ int excluded_from_list(const char *pathname,
return to_exclude;
}
}
+
+ if (!dtype) { /* matching against index */
+ basename = strrchr(pathname, '/');
+ if (basename) {
+ pathlen = basename-pathname;
+ memcpy(buf, pathname, pathlen);
+ buf[pathlen] = '\0';
+ pathname = buf;
+ basename = strrchr(pathname, '/');
+ basename = (basename) ? basename+1 : pathname;
+ goto recheck;
+ }
+ }
}
}
return -1; /* undecided */
diff --git a/t/t1009-read-tree-sparse-checkout.sh b/t/t1009-read-tree-sparse-checkout.sh
index 62246db..b57d237 100755
--- a/t/t1009-read-tree-sparse-checkout.sh
+++ b/t/t1009-read-tree-sparse-checkout.sh
@@ -84,13 +84,13 @@ cat >expected.swt <<EOF
H init.t
H sub/added
EOF
-test_expect_failure 'match directories without trailing slash' '
+test_expect_success 'match directories without trailing slash' '
echo init.t > .git/info/sparse-checkout &&
echo sub >> .git/info/sparse-checkout &&
git read-tree -m -u HEAD &&
git ls-files -t > result &&
test_cmp expected.swt result &&
- test ! -f init.t &&
+ test -f init.t &&
test -f sub/added
'
--
1.6.5.2.216.g9c1ec
^ permalink raw reply related
* [PATCH resend] gitk: Fix "git gui blame" invocation when called from topdir
From: Markus Heidelberg @ 2009-10-31 12:09 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git, Markus Heidelberg
In this case "git rev-parse --git-dir" doesn't return an absolute path,
but merely ".git", so the selected file has a relative path.
The function make_relative then tries to make the already relative path
relative, which results in a path like "../../../../Makefile" with as
much ".." as the number of parts [pwd] consists of.
This regression was introduced by commit 9712b81 (gitk: Fix bugs in
blaming code, 2008-12-06), which fixed "git gui blame" when called from
subdirs.
This also fixes it for bare repositories.
Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---
gitk | 30 +++++++++++++++++-------------
1 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/gitk b/gitk
index a0214b7..07a9440 100755
--- a/gitk
+++ b/gitk
@@ -3377,21 +3377,25 @@ proc index_sha1 {fname} {
# Turn an absolute path into one relative to the current directory
proc make_relative {f} {
- set elts [file split $f]
- set here [file split [pwd]]
- set ei 0
- set hi 0
- set res {}
- foreach d $here {
- if {$ei < $hi || $ei >= [llength $elts] || [lindex $elts $ei] ne $d} {
- lappend res ".."
- } else {
- incr ei
+ if {[file pathtype $f] ne "relative"} {
+ set elts [file split $f]
+ set here [file split [pwd]]
+ set ei 0
+ set hi 0
+ set res {}
+ foreach d $here {
+ if {$ei < $hi || $ei >= [llength $elts] || [lindex $elts $ei] ne $d} {
+ lappend res ".."
+ } else {
+ incr ei
+ }
+ incr hi
}
- incr hi
+ set elts [concat $res [lrange $elts $ei end]]
+ return [eval file join $elts]
+ } else {
+ return $f
}
- set elts [concat $res [lrange $elts $ei end]]
- return [eval file join $elts]
}
proc external_blame {parent_idx {line {}}} {
--
1.6.5.2.155.gaa0e5
^ permalink raw reply related
* ef/msys-imap, was Re: What's cooking in git.git (Oct 2009, #06; Fri, 30)
From: Johannes Schindelin @ 2009-10-31 12:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vljis9pks.fsf@alter.siamese.dyndns.org>
Hi,
On Fri, 30 Oct 2009, Junio C Hamano wrote:
> * ef/msys-imap (2009-10-22) 9 commits.
> - Windows: use BLK_SHA1 again
> - MSVC: Enable OpenSSL, and translate -lcrypto
> - mingw: enable OpenSSL
> - mingw: wrap SSL_set_(w|r)fd to call _get_osfhandle
> - imap-send: build imap-send on Windows
> - imap-send: fix compilation-error on Windows
> - imap-send: use run-command API for tunneling
> - imap-send: use separate read and write fds
> - imap-send: remove useless uid code
>
> This is pulled from J6t; I'll merge it to 'next' if Dscho is Ok with it.
Dscho is Ok with it.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 10/19] Allow helpers to request marks for fast-import
From: Johan Herland @ 2009-10-31 12:04 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: git, Johannes Schindelin, Daniel Barkalow
In-Reply-To: <fabb9a1e0910300526v5cbcf685l69f60c58b7e3732@mail.gmail.com>
On Friday 30 October 2009, Sverre Rabbelier wrote:
> Heya,
>
> On Fri, Oct 30, 2009 at 01:21, Johan Herland <johan@herland.net> wrote:
> > Please drop this patch from the series. The functionality is not
> > needed, since we'll use the fast-import "option" command from the
> > sr/gfi-options series instead.
>
> In that case I will rebase the series on top of sr/gfi-options then as
> soon as I reroll that one.
Good.
> Also, do you need to change anything else in git-remote-cvs to do that?
Yes, the sr/gfi-options series does cause some changes, both in git-remote-
cvs, and in the support libraries (adding a couple of methods to the
GitFastImport class in git.py).
This conglomeration of patch series is becoming fairly complicated, and it's
becoming hard to stay in sync. I suggest that you drop the CVS-specific
parts from this series, and work on stabilizing the common infrastructure.
Once that has settled, you can send a git-remote-hg series, and I can send a
rebased and updated git-remote-cvs series.
Feel free to reorganize the patches so that the git_remote_helpers
infrastructure is created in the correct location (instead of reorganizing
git_remote_cvs).
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [PATCH] diff --color-words -U0: fix the location of hunk headers
From: Johannes Schindelin @ 2009-10-31 11:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: markus.heidelberg, git
In-Reply-To: <7vr5sklo7c.fsf@alter.siamese.dyndns.org>
Hi,
On Fri, 30 Oct 2009, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > The reason I did not do that was to avoid a full subroutine call, as I
> > expected this code path to be very expensive.
>
> This is only done for the "word diff" mode, and my gut feeling is that it
> is not such a big issue.
Yeah, sorry, I should have stated explicitely that I no longer think that
there is a performance issue.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] clone: detect extra arguments
From: Johan Herland @ 2009-10-31 11:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Jonathan Nieder
In-Reply-To: <20091030145108.GA881@coredump.intra.peff.net>
On Friday 30 October 2009, Jeff King wrote:
> On Fri, Oct 30, 2009 at 10:45:25AM -0400, Jeff King wrote:
> > But looking at the usage message, there is some potential for cleanup.
>
> Also, we should probably do this (I did it as a patch on master, though,
> as it is an independent fix):
>
> -- >8 --
> Subject: [PATCH] clone: fix --recursive usage message
>
> Looks like a mistaken cut-and-paste in e7fed18a.
Yes. Please fix my screwup.
> Signed-off-by: Jeff King <peff@peff.net>
Acked-by: Johan Herland <johan@herland.net>
> ---
> builtin-clone.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/builtin-clone.c b/builtin-clone.c
> index 5762a6f..436e8da 100644
> --- a/builtin-clone.c
> +++ b/builtin-clone.c
> @@ -61,7 +61,7 @@ static struct option builtin_clone_options[] = {
> OPT_BOOLEAN('s', "shared", &option_shared,
> "setup as shared repository"),
> OPT_BOOLEAN(0, "recursive", &option_recursive,
> - "setup as shared repository"),
> + "initialize submodules in the clone"),
> OPT_STRING(0, "template", &option_template, "path",
> "path the template repository"),
> OPT_STRING(0, "reference", &option_reference, "repo",
>
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [PATCH] Don't create the $GIT_DIR/branches directory on init
From: Thomas Rast @ 2009-10-31 9:11 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: Junio C Hamano, git, spearce, sasa.zivkov
In-Reply-To: <1256923228-18949-1-git-send-email-robin.rosenberg@dewire.com>
Robin Rosenberg wrote:
> Git itself does not even look at this directory.
This contradicts the git-fetch manpage though: from urls-remotes.txt,
it includes
The name of one of the following can be used instead
of a URL as `<repository>` argument:
* a remote in the git configuration file: `$GIT_DIR/config`,
* a file in the `$GIT_DIR/remotes` directory, or
* a file in the `$GIT_DIR/branches` directory.
(and a longer explanation of what they need to look like).
So which one is wrong?
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* [PATCH v4] Teach git var about GIT_EDITOR
From: Jonathan Nieder @ 2009-10-31 7:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
In-Reply-To: <20091031045358.GA9565@progeny.tock>
Expose the command used by launch_editor() for scripts to use.
This should allow one to avoid searching for a proper editor
separately in each command.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
There was another typo in the patch I sent. The paper-bag fix:
diff -u b/var.c b/var.c
--- b/var.c
+++ b/var.c
@@ -35,7 +35,7 @@
const char *val;
for (ptr = git_vars; ptr->read; ptr++)
- if ((val = ptr->read(0))
+ if ((val = ptr->read(0)))
printf("%s=%s\n", ptr->name, val);
}
Here’s an updated patch. This one shouldn’t have any bugs (yeah, right).
Good night again,
Jonathan
Documentation/git-var.txt | 8 ++++++++
cache.h | 1 +
editor.c | 14 ++++++++++++--
var.c | 16 +++++++++++++++-
4 files changed, 36 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-var.txt b/Documentation/git-var.txt
index e2f4c09..89e4b4f 100644
--- a/Documentation/git-var.txt
+++ b/Documentation/git-var.txt
@@ -36,6 +36,14 @@ GIT_AUTHOR_IDENT::
GIT_COMMITTER_IDENT::
The person who put a piece of code into git.
+GIT_EDITOR::
+ Text editor for use by git commands. The value is meant to be
+ interpreted by the shell when it is used. Examples: `~/bin/vi`,
+ `$SOME_ENVIRONMENT_VARIABLE`, `"C:\Program Files\Vim\gvim.exe"
+ --nofork`. The order of preference is the `$GIT_EDITOR`
+ environment variable, then `core.editor` configuration, then
+ `$VISUAL`, then `$EDITOR`, and then finally 'vi'.
+
Diagnostics
-----------
You don't exist. Go away!::
diff --git a/cache.h b/cache.h
index 96840c7..311cfe1 100644
--- a/cache.h
+++ b/cache.h
@@ -750,6 +750,7 @@ extern const char *git_author_info(int);
extern const char *git_committer_info(int);
extern const char *fmt_ident(const char *name, const char *email, const char *date_str, int);
extern const char *fmt_name(const char *name, const char *email);
+extern const char *git_editor(void);
struct checkout {
const char *base_dir;
diff --git a/editor.c b/editor.c
index 3f13751..4f98b72 100644
--- a/editor.c
+++ b/editor.c
@@ -2,7 +2,7 @@
#include "strbuf.h"
#include "run-command.h"
-int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
+const char *git_editor(void)
{
const char *editor = getenv("GIT_EDITOR");
const char *terminal = getenv("TERM");
@@ -16,11 +16,21 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en
editor = getenv("EDITOR");
if (!editor && terminal_is_dumb)
- return error("terminal is dumb, but EDITOR unset");
+ return NULL;
if (!editor)
editor = "vi";
+ return editor;
+}
+
+int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
+{
+ const char *editor = git_editor();
+
+ if (!editor)
+ return error("terminal is dumb, but EDITOR unset");
+
if (strcmp(editor, ":")) {
size_t len = strlen(editor);
int i = 0;
diff --git a/var.c b/var.c
index dacbaab..a303757 100644
--- a/var.c
+++ b/var.c
@@ -8,6 +8,16 @@
static const char var_usage[] = "git var [-l | <variable>]";
+static const char *editor(int flag)
+{
+ const char *pgm = git_editor();
+
+ if (!pgm && flag & IDENT_ERROR_ON_NO_NAME)
+ die("terminal is dumb, but EDITOR unset");
+
+ return pgm;
+}
+
struct git_var {
const char *name;
const char *(*read)(int);
@@ -15,14 +25,18 @@ struct git_var {
static struct git_var git_vars[] = {
{ "GIT_COMMITTER_IDENT", git_committer_info },
{ "GIT_AUTHOR_IDENT", git_author_info },
+ { "GIT_EDITOR", editor },
{ "", NULL },
};
static void list_vars(void)
{
struct git_var *ptr;
+ const char *val;
+
for (ptr = git_vars; ptr->read; ptr++)
- printf("%s=%s\n", ptr->name, ptr->read(0));
+ if ((val = ptr->read(0)))
+ printf("%s=%s\n", ptr->name, val);
}
static const char *read_var(const char *var)
--
1.6.5.2
^ permalink raw reply related
* [PATCH v2 2/8] Do not use VISUAL editor on dumb terminals
From: Jonathan Nieder @ 2009-10-31 7:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
In-Reply-To: <20091031013039.GC5160@progeny.tock>
Jonathan Nieder wrote:
> Refuse to use $VISUAL and fall back to $EDITOR if TERM is unset
> or set to "dumb". Traditionally, VISUAL is set to a screen
> editor and EDITOR to a line-based editor, which should be more
> useful in that situation.
I was too lazy to wait for tests to finish on this one, and lo and
behold, they did not pass.
These additional changes seem to help, and they also add a test to
explain the change in editor behavior. The patch with these changes
squashed is also included in this message, below the scissors mark.
In the controlled environment used for tests, TERM is set to dumb
and ever since commit 02b3566 (test-lib.sh: Add a test_set_editor
function to safely set $VISUAL, 2008-05-04), most tests set VISUAL
when they want to set an editor for git to use. With this patch, they
should be using EDITOR instead.
--- a/t/t7005-editor.sh
+++ b/t/t7005-editor.sh
@@ -42,6 +42,16 @@ test_expect_success 'dumb should error out when falling back on vi' '
fi
'
+test_expect_success 'dumb should prefer EDITOR to VISUAL' '
+
+ EDITOR=./e-EDITOR.sh &&
+ VISUAL=./e-VISUAL.sh &&
+ export EDITOR VISUAL &&
+ git commit --amend &&
+ test "$(git show -s --format=%s)" = "Edited by EDITOR"
+
+'
+
TERM=vt100
export TERM
for i in vi EDITOR VISUAL core_editor GIT_EDITOR
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -86,7 +86,7 @@ chmod 755 editor
test_expect_success \
"amend commit" \
- "VISUAL=./editor git commit --amend"
+ "EDITOR=./editor git commit --amend"
test_expect_success \
"passing -m and -F" \
@@ -107,7 +107,7 @@ chmod 755 editor
test_expect_success \
"editing message from other commit" \
"echo 'hula hula' >file && \
- VISUAL=./editor git commit -c HEAD^ -a"
+ EDITOR=./editor git commit -c HEAD^ -a"
test_expect_success \
"message from stdin" \
@@ -141,10 +141,10 @@ EOF
test_expect_success \
'editor not invoked if -F is given' '
echo "moo" >file &&
- VISUAL=./editor git commit -a -F msg &&
+ EDITOR=./editor git commit -a -F msg &&
git show -s --pretty=format:"%s" | grep -q good &&
echo "quack" >file &&
- echo "Another good message." | VISUAL=./editor git commit -a -F - &&
+ echo "Another good message." | EDITOR=./editor git commit -a -F - &&
git show -s --pretty=format:"%s" | grep -q good
'
# We could just check the head sha1, but checking each commit makes it
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -30,7 +30,7 @@ TZ=UTC
TERM=dumb
export LANG LC_ALL PAGER TERM TZ
EDITOR=:
-VISUAL=:
+unset VISUAL
unset GIT_EDITOR
unset AUTHOR_DATE
unset AUTHOR_EMAIL
@@ -58,7 +58,7 @@ GIT_MERGE_VERBOSITY=5
export GIT_MERGE_VERBOSITY
export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
-export EDITOR VISUAL
+export EDITOR
GIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}
# Protect ourselves from common misconfiguration to export
@@ -207,8 +207,8 @@ trap 'die' EXIT
test_set_editor () {
FAKE_EDITOR="$1"
export FAKE_EDITOR
- VISUAL='"$FAKE_EDITOR"'
- export VISUAL
+ EDITOR='"$FAKE_EDITOR"'
+ export EDITOR
}
test_tick () {
-- %< --
Subject: [PATCH] Do not use VISUAL editor on dumb terminals
Refuse to use $VISUAL and fall back to $EDITOR if TERM is unset
or set to "dumb". Traditionally, VISUAL is set to a screen
editor and EDITOR to a line-based editor, which should be more
useful in that situation.
vim, for example, is happy to assume a terminal supports ANSI
sequences even if TERM is dumb (e.g., when running from a text
editor like Acme). git already refuses to fall back to vi on a
dumb terminal if GIT_EDITOR, core.editor, VISUAL, and EDITOR are
unset, but without this patch, that check is suppressed by
VISUAL=vi.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Jonathan Nieder <jrn@progeny.tock>
---
editor.c | 12 ++++++------
t/t7005-editor.sh | 10 ++++++++++
t/t7501-commit.sh | 8 ++++----
t/test-lib.sh | 8 ++++----
4 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/editor.c b/editor.c
index 941c0b2..3f13751 100644
--- a/editor.c
+++ b/editor.c
@@ -4,19 +4,19 @@
int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
{
- const char *editor, *terminal;
+ const char *editor = getenv("GIT_EDITOR");
+ const char *terminal = getenv("TERM");
+ int terminal_is_dumb = !terminal || !strcmp(terminal, "dumb");
- editor = getenv("GIT_EDITOR");
if (!editor && editor_program)
editor = editor_program;
- if (!editor)
+ if (!editor && !terminal_is_dumb)
editor = getenv("VISUAL");
if (!editor)
editor = getenv("EDITOR");
- terminal = getenv("TERM");
- if (!editor && (!terminal || !strcmp(terminal, "dumb")))
- return error("Terminal is dumb but no VISUAL nor EDITOR defined.");
+ if (!editor && terminal_is_dumb)
+ return error("terminal is dumb, but EDITOR unset");
if (!editor)
editor = "vi";
diff --git a/t/t7005-editor.sh b/t/t7005-editor.sh
index b647957..a95fe19 100755
--- a/t/t7005-editor.sh
+++ b/t/t7005-editor.sh
@@ -42,6 +42,16 @@ test_expect_success 'dumb should error out when falling back on vi' '
fi
'
+test_expect_success 'dumb should prefer EDITOR to VISUAL' '
+
+ EDITOR=./e-EDITOR.sh &&
+ VISUAL=./e-VISUAL.sh &&
+ export EDITOR VISUAL &&
+ git commit --amend &&
+ test "$(git show -s --format=%s)" = "Edited by EDITOR"
+
+'
+
TERM=vt100
export TERM
for i in vi EDITOR VISUAL core_editor GIT_EDITOR
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index d2de576..a603f6d 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -86,7 +86,7 @@ chmod 755 editor
test_expect_success \
"amend commit" \
- "VISUAL=./editor git commit --amend"
+ "EDITOR=./editor git commit --amend"
test_expect_success \
"passing -m and -F" \
@@ -107,7 +107,7 @@ chmod 755 editor
test_expect_success \
"editing message from other commit" \
"echo 'hula hula' >file && \
- VISUAL=./editor git commit -c HEAD^ -a"
+ EDITOR=./editor git commit -c HEAD^ -a"
test_expect_success \
"message from stdin" \
@@ -141,10 +141,10 @@ EOF
test_expect_success \
'editor not invoked if -F is given' '
echo "moo" >file &&
- VISUAL=./editor git commit -a -F msg &&
+ EDITOR=./editor git commit -a -F msg &&
git show -s --pretty=format:"%s" | grep -q good &&
echo "quack" >file &&
- echo "Another good message." | VISUAL=./editor git commit -a -F - &&
+ echo "Another good message." | EDITOR=./editor git commit -a -F - &&
git show -s --pretty=format:"%s" | grep -q good
'
# We could just check the head sha1, but checking each commit makes it
diff --git a/t/test-lib.sh b/t/test-lib.sh
index f2ca536..ec3336a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -30,7 +30,7 @@ TZ=UTC
TERM=dumb
export LANG LC_ALL PAGER TERM TZ
EDITOR=:
-VISUAL=:
+unset VISUAL
unset GIT_EDITOR
unset AUTHOR_DATE
unset AUTHOR_EMAIL
@@ -58,7 +58,7 @@ GIT_MERGE_VERBOSITY=5
export GIT_MERGE_VERBOSITY
export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
-export EDITOR VISUAL
+export EDITOR
GIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}
# Protect ourselves from common misconfiguration to export
@@ -207,8 +207,8 @@ trap 'die' EXIT
test_set_editor () {
FAKE_EDITOR="$1"
export FAKE_EDITOR
- VISUAL='"$FAKE_EDITOR"'
- export VISUAL
+ EDITOR='"$FAKE_EDITOR"'
+ export EDITOR
}
test_tick () {
--
1.6.5.2
>
> vim, for example, is happy to assume a terminal supports ANSI
> sequences even if TERM is dumb (e.g., when running from a text
> editor like Acme). git already refuses to fall back to vi on a
> dumb terminal if GIT_EDITOR, core.editor, VISUAL, and EDITOR are
> unset, but without this patch, that check is suppressed by
> VISUAL=vi.
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> This patch eases my discomfort about the error message a little. It
> is not actually needed to support any ways of working I engage in.
>
> If stdout is redirected, this is probably still making the wrong
> choice; isatty(STDOUT_FILENO) might be a more useful datum to use.
> But it does not seem worth complicating the logic further.
>
> editor.c | 12 ++++++------
> 1 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/editor.c b/editor.c
> index 941c0b2..3f13751 100644
> --- a/editor.c
> +++ b/editor.c
> @@ -4,19 +4,19 @@
>
> int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
> {
> - const char *editor, *terminal;
> + const char *editor = getenv("GIT_EDITOR");
> + const char *terminal = getenv("TERM");
> + int terminal_is_dumb = !terminal || !strcmp(terminal, "dumb");
>
> - editor = getenv("GIT_EDITOR");
> if (!editor && editor_program)
> editor = editor_program;
> - if (!editor)
> + if (!editor && !terminal_is_dumb)
> editor = getenv("VISUAL");
> if (!editor)
> editor = getenv("EDITOR");
>
> - terminal = getenv("TERM");
> - if (!editor && (!terminal || !strcmp(terminal, "dumb")))
> - return error("Terminal is dumb but no VISUAL nor EDITOR defined.");
> + if (!editor && terminal_is_dumb)
> + return error("terminal is dumb, but EDITOR unset");
>
> if (!editor)
> editor = "vi";
> --
> 1.6.5.2
>
^ permalink raw reply related
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