Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Add commit.editor configuration variable
From: Johannes Schindelin @ 2007-07-19  9:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Shawn O. Pearce, Adam Roben, git
In-Reply-To: <7v1wf451fc.fsf@assigned-by-dhcp.cox.net>

Hi,

On Wed, 18 Jul 2007, Junio C Hamano wrote:

> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > We only launch an editor for three reasons: commit messages, tag
> > messages and git-rebase -i.  If we were to ever add a new editor
> > using thingy, odds are the user would want the same editor by
> > default for that too.
> >
> > So please, core.editor, and also use it in git-rebase--interactive.
> 
> Ah, add "git-am -i" to the mix.  Potentially, git-notes would
> use it as well.

Now with so many commands in the lot, how about putting the code into 
git-sh-setup, into a function "get_editor()"?

Ciao,
Dscho

^ permalink raw reply

* Re: [REVISED PATCH 2/6] Introduce commit notes
From: Sven Verdoolaege @ 2007-07-19  9:54 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, Alberto Bertogli, git, Johan Herland
In-Reply-To: <Pine.LNX.4.64.0707190258550.14781@racer.site>

On Thu, Jul 19, 2007 at 03:30:43AM +0100, Johannes Schindelin wrote:
> +If such a path exists in the given ref, the referenced blob is read, and
> +appended to the commit message, separated by a "Notes:" line.  If the
> +given ref itself does not exist, it is not an error, but means that no
> +notes should be print.

printed?

skimo

^ permalink raw reply

* Re: [REVISED PATCH 2/6] Introduce commit notes
From: Johannes Schindelin @ 2007-07-19  9:50 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, Alberto Bertogli, git, Johan Herland
In-Reply-To: <alpine.LFD.0.999.0707181949490.27353@woody.linux-foundation.org>

Hi,

On Wed, 18 Jul 2007, Linus Torvalds wrote:

> On Thu, 19 Jul 2007, Johannes Schindelin wrote:
> >
> > There is one severe shortcoming, though.  Since tree objects can 
> > contain file names of a variable length, it is not possible to do a 
> > binary search for the correct base name in the tree object's contents.
> 
> Well, I've been thinking about this, and that's not really entirely 
> correct.
> 
> It *is* possible to do a binary search, it's just a bit complicated, 
> because you have to take the "halfway" thing, and find the beginning of 
> an entry.

I will try to work from your proposal, and do some timings.  But for the 
notes, I really, really like the average constant running time of the hash 
map.  As you can see from my timings in

http://thread.gmane.org/gmane.comp.version-control.git/52598/focus=52603

it does make a difference, compared to binary search.

Ciao,
Dscho

^ permalink raw reply

* Re: [REVISED PATCH 2/6] Introduce commit notes
From: Junio C Hamano @ 2007-07-19  9:34 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Johannes Schindelin, Alberto Bertogli, git, Johan Herland
In-Reply-To: <7vfy3l3rj0.fsf@assigned-by-dhcp.cox.net>

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

> Linus Torvalds <torvalds@linux-foundation.org> writes:
> ...
>> So if you're really really *really* unlucky, you might end up having to 
>> fall back on the linear search. But it still works!
>>
>> Can anybody see anything wrong in my thinking above?
> ...
> But the real problem of this approach of course is that this is
> not reliable and can get a false match.  You can find your
> beginning NUL in the SHA-1 part of one entry, and terminating
> NUL later in the SHA-1 part of next entry, and you will never
> notice.

In other words, if you are really really *really* unlucky, not
only you might end up being fooled by random byte sequences in
SHA-1 part of the tree object, you would not even notice that
you have to fall back on the linear search.

I've long time ago concluded that if we care about reliability
(and we do very much), a bisectable tree without breaking
backward compatibility is impossible.  I was hoping to find a
"hole" in tree object format so that I can place an extended
section that is invisible to older versions of git, and place a
table that records offsets of each tree entries to help
bisection and/or perhaps a hash table to help look-up, but I do
not think it is possible.  In the case of index file, the
original file format had a hole after the cache-entry array
where we can later squeeze an extension section that is
invisible to older versions of git.  But the tree object format
is designed so tight that I do not see there is any place to put
an extension section.

Side note: I also think adding "extension section" to tree
object is not a good idea to begin with.  The data nor length of
such a section cannot participate in hash computation to derive
the tree's object name so that we can still compare two tree
objects (with and without such extension) that have the same
contents by only looking at their object names.  But having
contents that are not counted as parts of the object's name goes
against the reliability and safety of git.

> ...
> I was suggesting to have a specialized parser only to read such
> tree objects that are "abused" to represent notes.  You can
> cheaply validate that these trees are of expected shape.
> ...
> For an added safety, a "notes" writer could even throw in
> signature bytes (say, a symlink whose name is " !" in the
> top-level tree, and another symlink " !{37}" in the second-level
> tree) to protect the reader.

Of course, even with the above trick with relatively cheap
validation based on size, entry format, and "signature entries",
the way I outlined to speed up "notes" access really relies on
the tree objects used in "notes" to be well formed.  If somebody
throws in a tree that is not really a "note" to refs/notes/, and
if I am really really *really* unlucky, not only I might end up
being fooled by random byte sequences in SHA-1 part of the tree
object, I would not even notice that I am reading garbage and
end up giving garbage as "note" to the object back to the user.

^ permalink raw reply

* Re: [REVISED PATCH 2/6] Introduce commit notes
From: Johannes Schindelin @ 2007-07-19  9:24 UTC (permalink / raw)
  To: Wincent Colaiuta; +Cc: Junio C Hamano, Alberto Bertogli, git, Johan Herland
In-Reply-To: <E4F64312-3F86-49F3-B6BD-D148AFBAB520@wincent.com>

Hi,

On Thu, 19 Jul 2007, Wincent Colaiuta wrote:

> El 19/7/2007, a las 4:30, Johannes Schindelin escribi?:
> 
> > Commit notes are blobs which are shown together with the commit
> > message.  These blobs are taken from the notes ref, which you can
> > configure by the config variable core.notesRef, which in turn can
> > be overridden by the environment variable GIT_NOTES_REF.
> 
> I was trying to look back and find out what the rationale/usage scenario for
> these commit notes might be but Googling for 'git "commit notes"' doesn't
> turn up much other than the original patch you sent a few days ago.
> 
> Is this an evolution of the "git-note: A mechanisim for providing free-form
> after-the-fact annotations on commits" first introduced here?:
> 
> <http://lists.zerezo.com/git/msg465441.html>

Almost.  It is an evolution of the evolution of this.

http://thread.gmane.org/gmane.comp.version-control.git/52598/focus=52603

(which started this thread you were replying to) hints at that, but you're 
right, I failed to give an explicit reference:

http://article.gmane.org/gmane.comp.version-control.git/49588

Background: It was discussed how to go about storing notes (in the mail 
you cited).  I was convinced that Johan's 15-strong patch series was not 
optimal, in that it tried to introduce a _second_ object store, 
_exclusively_ for commit notes, with all kinds of problems like "how to 
fetch it?".

After thinking about how to avoid duplicating the object store, I posted 
my proposal, in the second link I gave.

It was shot down, because of scalability problems.  They were not serious, 
but hurt enough that I stalled working on it, until Alberto reminded me.

Since I felt bad about shooting down Johan's patch series, and then not 
completing my alternative solution, I ended up working on it some more.  
The WIP patch 6/6 hints at what I will submit in the next days, to speed 
up in a transparent manner what would otherwise not scale well.

Ciao,
Dscho

^ permalink raw reply

* Re: [REVISED PATCH 2/6] Introduce commit notes
From: Wincent Colaiuta @ 2007-07-19  9:05 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, Alberto Bertogli, git, Johan Herland
In-Reply-To: <Pine.LNX.4.64.0707190258550.14781@racer.site>

El 19/7/2007, a las 4:30, Johannes Schindelin escribió:

> Commit notes are blobs which are shown together with the commit
> message.  These blobs are taken from the notes ref, which you can
> configure by the config variable core.notesRef, which in turn can
> be overridden by the environment variable GIT_NOTES_REF.

I was trying to look back and find out what the rationale/usage  
scenario for these commit notes might be but Googling for 'git  
"commit notes"' doesn't turn up much other than the original patch  
you sent a few days ago.

Is this an evolution of the "git-note: A mechanisim for providing  
free-form after-the-fact annotations on commits" first introduced here?:

<http://lists.zerezo.com/git/msg465441.html>

Cheers,
Wincent

^ permalink raw reply

* Re: [RFC PATCH] Re: Empty directories...
From: Matthieu Moy @ 2007-07-19  8:13 UTC (permalink / raw)
  To: Shawn O. Pearce
  Cc: Junio C Hamano, Linus Torvalds, David Kastrup,
	Johannes Schindelin, Git Mailing List
In-Reply-To: <20070719060922.GF32566@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> If I do:
>
>   mkdir -p foo/bar
>   echo hello >foo/bar/world
>   git add foo
>   git -f rm foo/bar/world
>
> I never asked for foo/bar or foo to stay.

Well, outside git, if you do

$ mkdir -p foo/bar
$ echo hello > foo/bar/world
$ rm -f foo/bar/world

You didn't ask foo/bar to stay either, and still, it's quite natural
to have it stay in your filesystem. So, the same way you'd have ran
"rm -r foo", it seems reasonable to me to ask for "git-rm -r foo" if
the user wants to get rid of foo/ itself.

-- 
Matthieu

^ permalink raw reply

* Re: [PATCH (REVISED)] Add core.editor configuration variable
From: Andy Parkins @ 2007-07-19  7:48 UTC (permalink / raw)
  To: git; +Cc: Adam Roben, Junio C Hamano
In-Reply-To: <11848281302504-git-send-email-aroben@apple.com>

On Thursday 2007 July 19, Adam Roben wrote:

>    Well, it turns out we already do launch an editor in other places,
> namely "git am -i" and "git send-email --compose". So, this patch takes
> care of those cases as well.

Perhaps I'm being overly pedantic, but it seems odd to put options that are 
relevant only to porcelain under the "core" section.

core.pager is in the same category - but that's already in.

Would porcelain.editor be a better name for this variable?


Andy

-- 
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com

^ permalink raw reply

* Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats
From: Luben Tuikov @ 2007-07-19  7:40 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Junio C Hamano, Matt McCutchen, git, Petr Baudis
In-Reply-To: <200707190930.57019.jnareb@gmail.com>

--- Jakub Narebski <jnareb@gmail.com> wrote:
> Luben Tuikov wrote:
> 
> > I wouldn't mind an improvement in the snapshot area of gitweb.
> > I wasn't really happy with the snapshot feature as it was originally
> > implemented, as it would generate a tar file with ".tar.bz2"
> > name extension, but the file was NOT bz2, and I had to always
> > manually rename, bz2, and rename back.
> 
> This was a *bug*, but it is now corrected (in 9aa17573). Gitweb used 
> Content-Encoding, which is meant for _transparent_ compression.

Yeah, that's what I suspected, since there was nothing obviously
wrong with the code.

Thanks for the fix.

   Luben

^ permalink raw reply

* Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats
From: Jakub Narebski @ 2007-07-19  7:30 UTC (permalink / raw)
  To: ltuikov; +Cc: Junio C Hamano, Matt McCutchen, git, Petr Baudis
In-Reply-To: <571532.59758.qm@web31813.mail.mud.yahoo.com>

Luben Tuikov wrote:

> I wouldn't mind an improvement in the snapshot area of gitweb.
> I wasn't really happy with the snapshot feature as it was originally
> implemented, as it would generate a tar file with ".tar.bz2"
> name extension, but the file was NOT bz2, and I had to always
> manually rename, bz2, and rename back.

This was a *bug*, but it is now corrected (in 9aa17573). Gitweb used 
Content-Encoding, which is meant for _transparent_ compression.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [RFC PATCH] Re: Empty directories...
From: Geoff Russell @ 2007-07-19  7:10 UTC (permalink / raw)
  To: git
In-Reply-To: <85vechudrl.fsf@lola.goethe.zz>

Dear gits,

When I first started using git, I naively did

           $ mkdir NEWDIR && chmod BLAH NEWDIR
           $ git add NEWDIR

I just expected that this was content in the current directory that I
wanted tracked
together with the permissions.

It wasn't ... I spent a day or 2 thinking I was stupid, my version of git was
corrupt, my machine was busted, .... etc.  Eventually of course, I read the
documentation (when all else fails) and realised that this perfectly obvious
behaviour was not supported.  The behaviour was obviously so obvious
that eventually
an error message was added telling all the people who hadn't
read the documentation that trying to add a directory was 'fatal'.

I put up with and work around this behaviour because git is so bloody
brilliant at everything else.  But it would be nice if it worked.

Cheers,
Geoff Russell

^ permalink raw reply

* Re: Another question about importing SVN with fast-import
From: David Frech @ 2007-07-19  7:09 UTC (permalink / raw)
  To: Julian Phillips; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0707162204480.14971@reaper.quantumfyre.co.uk>

On 7/16/07, Julian Phillips <julian@quantumfyre.co.uk> wrote:
> Now the question.  Shawn recently added C and R operations - almost as
> soon as they were asked for too.  However, how do you copy a file from a
> particular revision?  I have just hit a point where someone deleted a
> directory, and then copied one of the files from that directory back from
> an old revision (as two separate commits).  Since I'm not tracking any
> branch contents in my front-end, and the copy operation only works from
> the current branch head I seem to be stuck ... or have I missed something?

I have to second Julian's question.

The only things I have left to implement (in my svn dump to
fast-import translator) before I can call my code "complete"  are
branch copies (which will generate "from" lines in commits so
fast-import can properly initialize the new branch) and a generic
"copy <path> from <rev>" that Julian also needs.

I mark my commits and keep track of the mapping from svn revs to git
commits, so when the svn dump asks for a <path> from a specific <svn
rev> I can correlate that to a mark (ie, a git commit). But I can't
tell fast-import to *do* anything with it.

This is kind of a request and a comment/question. The request is:
there is no way to do *reasonably* in the front end what fast-import
can do somewhat reasonably: namely, copy a <path> (file or directory!)
from an arbitrary previously committed revision/mark to the current
branch.

The comment/question is: how different is this, really, from being
able to  specify a "from" line in a commit? In both cases I'm asking
fast-import to reach into its memory (or the repo) and pull out a
tree, and to add (some or all of it) to my current branch. Isn't the
kind of generic C command that Julian and I are asking for the same
thing, only instead of taking the whole tree (from the specified
commit) it takes a single file or directory?

I hope I haven't missed the point entirely.

Lastly, do we really need "R"? With this generic copy - and I think
there should be *only* a generic version, not a "streamlined local
copy" version and a "reach into history arbitrarily" version - we can,
as an earlier poster pointed out, do R by doing a C and then a D. This
is, in fact, how svn dump files represent file and directory renames.

It would be nice to keep the fast-import command set small and orthogonal.

My few cents.

Cheers,

- David

-- 
If I have not seen farther, it is because I have stood in the
footsteps of giants.

^ permalink raw reply

* [PATCH (REVISED)] Add core.editor configuration variable
From: Adam Roben @ 2007-07-19  6:55 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Adam Roben

This variable lets you specify an editor that will be launched in preference to
the EDITOR and VISUAL environment variables.

Signed-off-by: Adam Roben <aroben@apple.com>
---
On Jul 18, 2007, at 11:23 PM, Shawn O. Pearce wrote:
> We only launch an editor for three reasons: commit messages, tag
> messages and git-rebase -i.  If we were to ever add a new editor
> using thingy, odds are the user would want the same editor by
> default for that too.

   Well, it turns out we already do launch an editor in other places, namely
"git am -i" and "git send-email --compose". So, this patch takes care of those
cases as well.

   My original intent had been to have an editor that really was *just* for
commit messages, but I realize now that that is probably something that not too
many people want, whereas core.editor is much more generally useful. Thanks
Junio and Shawn for the advice.

 Documentation/git-commit.txt     |    9 +++++----
 Documentation/git-send-email.txt |    4 ++--
 git-am.sh                        |    2 +-
 git-commit.sh                    |   13 +++++++------
 git-rebase--interactive.sh       |    2 +-
 git-send-email.perl              |    3 +--
 git-tag.sh                       |    2 +-
 7 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index f96142f..5caad56 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -244,10 +244,11 @@ on the Subject: line and the rest of the commit in the body.
 
 include::i18n.txt[]
 
-ENVIRONMENT VARIABLES
----------------------
-The command specified by either the VISUAL or EDITOR environment
-variables is used to edit the commit log message.
+ENVIRONMENT AND CONFIGURATION VARIABLES
+---------------------------------------
+The editor used to edit the commit log message will be chosen from the
+core.editor configuration variable, the VISUAL environment variable, or the
+EDITOR environment variable (in that order).
 
 HOOKS
 -----
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 293686c..e7723c9 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -44,8 +44,8 @@ The --cc option must be repeated for each user you want on the cc list.
 	value; if that is unspecified, default to --chain-reply-to.
 
 --compose::
-	Use $EDITOR to edit an introductory message for the
-	patch series.
+	Use core.editor, $VISUAL, or $EDITOR to edit an introductory message
+	for the patch series.
 
 --from::
 	Specify the sender of the emails.  This will default to
diff --git a/git-am.sh b/git-am.sh
index e5e6f2c..3a651ae 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -364,7 +364,7 @@ do
 		[yY]*) action=yes ;;
 		[aA]*) action=yes interactive= ;;
 		[nN]*) action=skip ;;
-		[eE]*) "${VISUAL:-${EDITOR:-vi}}" "$dotest/final-commit"
+		[eE]*) "$(git config core.editor || echo ${VISUAL:-${EDITOR:-vi}})" "$dotest/final-commit"
 		       action=again ;;
 		[vV]*) action=again
 		       LESS=-S ${PAGER:-less} "$dotest/patch" ;;
diff --git a/git-commit.sh b/git-commit.sh
index 3f3de17..72e4cf0 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -544,18 +544,19 @@ fi
 
 case "$no_edit" in
 '')
-	case "${VISUAL:-$EDITOR},$TERM" in
+	commit_editor=$(git config core.editor || echo ${VISUAL:-$EDITOR})
+	case "$commit_editor,$TERM" in
 	,dumb)
-		echo >&2 "Terminal is dumb but no VISUAL nor EDITOR defined."
-		echo >&2 "Please supply the commit log message using either"
-		echo >&2 "-m or -F option.  A boilerplate log message has"
-		echo >&2 "been prepared in $GIT_DIR/COMMIT_EDITMSG"
+		echo >&2 "Terminal is dumb but core.editor, VISUAL, and EDITOR"
+		echo >&2 "are undefined. Please supply the commit log message"
+		echo >&2 "using either -m or -F option.  A boilerplate log message"
+		echo >&2 "has been prepared in $GIT_DIR/COMMIT_EDITMSG"
 		exit 1
 		;;
 	esac
 	git-var GIT_AUTHOR_IDENT > /dev/null  || die
 	git-var GIT_COMMITTER_IDENT > /dev/null  || die
-	${VISUAL:-${EDITOR:-vi}} "$GIT_DIR/COMMIT_EDITMSG"
+	${commit_editor:-vi} "$GIT_DIR/COMMIT_EDITMSG"
 	;;
 esac
 
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index f395076..2e15abb 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -414,7 +414,7 @@ EOF
 			die_abort "Nothing to do"
 
 		cp "$TODO" "$TODO".backup
-		${VISUAL:-${EDITOR:-vi}} "$TODO" ||
+		$(git config core.editor || echo ${VISUAL:-${EDITOR:-vi}}) "$TODO" ||
 			die "Could not execute editor"
 
 		test -z "$(grep -ve '^$' -e '^#' < $TODO)" &&
diff --git a/git-send-email.perl b/git-send-email.perl
index 7552cac..ad17b4e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -341,8 +341,7 @@ GIT: for the patch you are writing.
 EOT
 	close(C);
 
-	my $editor = $ENV{EDITOR};
-	$editor = 'vi' unless defined $editor;
+	my $editor = $repo->config("core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
 	system($editor, $compose_filename);
 
 	open(C2,">",$compose_filename . ".final")
diff --git a/git-tag.sh b/git-tag.sh
index 1c25d88..9aa30b4 100755
--- a/git-tag.sh
+++ b/git-tag.sh
@@ -177,7 +177,7 @@ if [ "$annotate" ]; then
         ( echo "#"
           echo "# Write a tag message"
           echo "#" ) > "$GIT_DIR"/TAG_EDITMSG
-        ${VISUAL:-${EDITOR:-vi}} "$GIT_DIR"/TAG_EDITMSG || exit
+        $(git config core.editor || echo ${VISUAL:-${EDITOR:-vi}}) "$GIT_DIR"/TAG_EDITMSG || exit
     else
         printf '%s\n' "$message" >"$GIT_DIR"/TAG_EDITMSG
     fi
-- 
1.5.3.rc2.19.gc4fba-dirty

^ permalink raw reply related

* Re: [PATCH] Add commit.editor configuration variable
From: Junio C Hamano @ 2007-07-19  6:53 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Adam Roben, git
In-Reply-To: <20070719062302.GG32566@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> We only launch an editor for three reasons: commit messages, tag
> messages and git-rebase -i.  If we were to ever add a new editor
> using thingy, odds are the user would want the same editor by
> default for that too.
>
> So please, core.editor, and also use it in git-rebase--interactive.

Ah, add "git-am -i" to the mix.  Potentially, git-notes would
use it as well.

^ permalink raw reply

* Re: [PATCH] Add commit.editor configuration variable
From: Shawn O. Pearce @ 2007-07-19  6:23 UTC (permalink / raw)
  To: Adam Roben; +Cc: Junio C Hamano, git
In-Reply-To: <47EE39C7-0D57-48EC-B5A0-C10E49997E32@apple.com>

Adam Roben <aroben@apple.com> wrote:
> On Jul 18, 2007, at 11:08 PM, Junio C Hamano wrote:
> 
> >I do not think commit.editor is a good name.  Wouldn't we want
> >that customized editor for "git tag -a" as well?  Probably
> >core.editor would come nicely next to core.pager we already
> >have.
> 
>    I considered core.editor, but if it's an editor that is *only*  
> used for commit messages then that seems to be a too-general name, and  
> something like core.commit_message_editor seemed far too long. Any  
> suggestions?
> 
>    I had forgotten about "git tag -a" -- I will add support for that  
> as well.

We only launch an editor for three reasons: commit messages, tag
messages and git-rebase -i.  If we were to ever add a new editor
using thingy, odds are the user would want the same editor by
default for that too.

So please, core.editor, and also use it in git-rebase--interactive.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] Add commit.editor configuration variable
From: Adam Roben @ 2007-07-19  6:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7iox3oz8.fsf@assigned-by-dhcp.cox.net>

On Jul 18, 2007, at 11:08 PM, Junio C Hamano wrote:

> I do not think commit.editor is a good name.  Wouldn't we want
> that customized editor for "git tag -a" as well?  Probably
> core.editor would come nicely next to core.pager we already
> have.

    I considered core.editor, but if it's an editor that is *only*  
used for commit messages then that seems to be a too-general name, and  
something like core.commit_message_editor seemed far too long. Any  
suggestions?

    I had forgotten about "git tag -a" -- I will add support for that  
as well.

-Adam

^ permalink raw reply

* Re: [RFC PATCH] Re: Empty directories...
From: Shawn O. Pearce @ 2007-07-19  6:09 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Linus Torvalds, David Kastrup, Matthieu Moy, Johannes Schindelin,
	Git Mailing List
In-Reply-To: <20070719053858.GE32566@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> wrote:
> Junio C Hamano <gitster@pobox.com> wrote:
> > Another issue I thought about was what you would do in the step
> > 3 in the following:
> > 
> >  1. David says "mkdir D; git add D"; you add S_IFDIR entry in
> >     the index at D;
> > 
> >  2. David says "date >D/F; git add D/F"; presumably you drop D
> >     from the index (to keep the index more backward compatible)
> >     and add S_IFREG entry at D/F.
> > 
> >  3. David says "git rm D/F".
> > 
> > Have we stopped keeping track of the "empty directory" at this
> > point?
> 
> Sadly yes.  But I don't think that's what the folks who want to
> track empty directories want to have happen here.
> 
> Which is why I'm thinking we just need to track the directory, as a
> node in the index, even if there are files in it, and even if we got
> that directory and its contained files there by just unpacking trees.

I take this back.  I really don't want that behavior.

If I do:

  mkdir -p foo/bar
  echo hello >foo/bar/world
  git add foo
  git -f rm foo/bar/world

I never asked for foo/bar or foo to stay.  In fact I want them
to disappear from Git entirely, as foo/bar is now empty and has
no content.


But we also cannot do a special --mkdir option for update-index
either, because how do we know that the user designated subtree is
a directory we must always keep in the index?

So I think the only way this works is to have a new mode that we use
in tree (04755 ?) that tells us not only is this thing a subtree,
but also that the user wants it to stay here, even if it is empty.
Those trees are always in the index as a real tree entry, even if
there are files contained in it.

And as far as getting that directory entry created/removed from
the index, well, I think a special flag to update-index would be
in order, much like --chmod=[+-]x.

Just my $0.0002 USD, which really ain't worth much at all.

-- 
Shawn.

^ permalink raw reply

* Re: [RFC PATCH] Re: Empty directories...
From: David Kastrup @ 2007-07-19  6:08 UTC (permalink / raw)
  To: git
In-Reply-To: <20070719053858.GE32566@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> Sadly yes.  But I don't think that's what the folks who want to
> track empty directories want to have happen here.
>
> Which is why I'm thinking we just need to track the directory, as a
> node in the index, even if there are files in it, and even if we got
> that directory and its contained files there by just unpacking
> trees.

I have come to about the same conclusion.  So if
backward-compatibility is any concern, one needs to work with some
sort of extension records, and designing them in a way that

new-git add tree
old-git rm tree

will not leave empty subdirectories in the index will be tricky, to
say the least.  One will likely have to add an extension record
"directory" for each directory as well as "my containing dir takes
care of itself" to each file that has been added with new-git and has
had its parent directory entered by other means.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply

* Re: [PATCH] Add commit.editor configuration variable
From: Junio C Hamano @ 2007-07-19  6:08 UTC (permalink / raw)
  To: Adam Roben; +Cc: git
In-Reply-To: <11848235881723-git-send-email-aroben@apple.com>

I do not think commit.editor is a good name.  Wouldn't we want
that customized editor for "git tag -a" as well?  Probably
core.editor would come nicely next to core.pager we already
have.

^ permalink raw reply

* Re: [RFC PATCH] Re: Empty directories...
From: David Kastrup @ 2007-07-19  5:59 UTC (permalink / raw)
  To: git
In-Reply-To: <7vbqe93qtv.fsf@assigned-by-dhcp.cox.net>

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

> Another issue I thought about was what you would do in the step
> 3 in the following:
>
>  1. David says "mkdir D; git add D"; you add S_IFDIR entry in
>     the index at D;
>
>  2. David says "date >D/F; git add D/F"; presumably you drop D
>     from the index (to keep the index more backward compatible)
>     and add S_IFREG entry at D/F.

I don't think that one should drop D here.  Operation 1 _is_ not
backward compatible, so if you want to revert it, you should
explicitly remove D.  And we can't "keep" the index backward
compatible if it isn't so after step 1.

>  3. David says "git rm D/F".
>
> Have we stopped keeping track of the "empty directory" at this
> point?

The case I am worrying about is rather

mkdir D
mkdir D/E
touch D/E/file
git add D
[*]
git rm D/E/file

>From a user perspective, E should be registered still.  Compare this
with

mkdir D
mkdir D/E
touch D/E/file
git add D/E/file
[*]
git rm D/E/file

Where likely both D and E should now be considered unregistered.  So
the situation is different between the first or the second [*], and
the difference might be impossible to express completely in the frame
of a backwards-compatible index, even though we don't track an empty
directory at the point [*] at all, and the only registered _file_ is
D/E/file.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply

* [PATCH] Add commit.editor configuration variable
From: Adam Roben @ 2007-07-19  5:39 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Adam Roben

This variable lets you specify a different editor for editing commit messages.
If commit.editor is not set, git-commit falls back to VISUAL, then EDITOR as
before.

Signed-off-by: Adam Roben <aroben@apple.com>
---
 Documentation/git-commit.txt |    9 +++++----
 git-commit.sh                |   13 +++++++------
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index f96142f..1a628be 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -244,10 +244,11 @@ on the Subject: line and the rest of the commit in the body.
 
 include::i18n.txt[]
 
-ENVIRONMENT VARIABLES
----------------------
-The command specified by either the VISUAL or EDITOR environment
-variables is used to edit the commit log message.
+ENVIRONMENT AND CONFIGURATION VARIABLES
+---------------------------------------
+The editor used to edit the commit log message will be chosen from the
+commit.editor configuration variable, the VISUAL environment variable, or the
+EDITOR environment variable (in that order).
 
 HOOKS
 -----
diff --git a/git-commit.sh b/git-commit.sh
index 3f3de17..c4d8501 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -544,18 +544,19 @@ fi
 
 case "$no_edit" in
 '')
-	case "${VISUAL:-$EDITOR},$TERM" in
+	commit_editor=$(git config commit.editor || echo ${VISUAL:-$EDITOR})
+	case "$commit_editor,$TERM" in
 	,dumb)
-		echo >&2 "Terminal is dumb but no VISUAL nor EDITOR defined."
-		echo >&2 "Please supply the commit log message using either"
-		echo >&2 "-m or -F option.  A boilerplate log message has"
-		echo >&2 "been prepared in $GIT_DIR/COMMIT_EDITMSG"
+		echo >&2 "Terminal is dumb but commit.editor, VISUAL, and EDITOR"
+		echo >&2 "are undefined. Please supply the commit log message"
+		echo >&2 "using either -m or -F option.  A boilerplate log message"
+		echo >&2 "has been prepared in $GIT_DIR/COMMIT_EDITMSG"
 		exit 1
 		;;
 	esac
 	git-var GIT_AUTHOR_IDENT > /dev/null  || die
 	git-var GIT_COMMITTER_IDENT > /dev/null  || die
-	${VISUAL:-${EDITOR:-vi}} "$GIT_DIR/COMMIT_EDITMSG"
+	${commit_editor:-vi} "$GIT_DIR/COMMIT_EDITMSG"
 	;;
 esac
 
-- 
1.5.2.2.620.g42358-dirty

^ permalink raw reply related

* Re: [RFC PATCH] Re: Empty directories...
From: Shawn O. Pearce @ 2007-07-19  5:38 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Linus Torvalds, David Kastrup, Matthieu Moy, Johannes Schindelin,
	Git Mailing List
In-Reply-To: <7vbqe93qtv.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <gitster@pobox.com> wrote:
> Another issue I thought about was what you would do in the step
> 3 in the following:
> 
>  1. David says "mkdir D; git add D"; you add S_IFDIR entry in
>     the index at D;
> 
>  2. David says "date >D/F; git add D/F"; presumably you drop D
>     from the index (to keep the index more backward compatible)
>     and add S_IFREG entry at D/F.
> 
>  3. David says "git rm D/F".
> 
> Have we stopped keeping track of the "empty directory" at this
> point?

Sadly yes.  But I don't think that's what the folks who want to
track empty directories want to have happen here.

Which is why I'm thinking we just need to track the directory, as a
node in the index, even if there are files in it, and even if we got
that directory and its contained files there by just unpacking trees.

-- 
Shawn.

^ permalink raw reply

* Re: [RFC PATCH] Re: Empty directories...
From: Junio C Hamano @ 2007-07-19  5:28 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: David Kastrup, Matthieu Moy, Johannes Schindelin,
	Git Mailing List
In-Reply-To: <alpine.LFD.0.999.0707181710271.27353@woody.linux-foundation.org>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Thu, 19 Jul 2007, David Kastrup wrote:
>> 
>> Well, kudos.  Together with the analysis from Junio, this seems like a
>> good start.  Would you have any recommendations about what stuff one
>> should really read in order to get up to scratch about git internals?
>
> Well, you do need to understand the index. That's where all the new 
> subtlety happens.
>
> The data structures themselves are trivial, and we've supported empty 
> trees (at the top level) from the beginning, so that part is not anything 
> new.
>
> However, now having a new entry type in the index (S_IFDIR) means that 
> anything that interacts with the index needs to think twice. But a lot of 
> that is just testing what happens, and so the first thing to do is to have 
> a test-suite.
>
> There's also the question about how to show an empty tree in a diff. We've 
> never had that: the only time we had empty trees was when we compared a 
> totally empty "root" tree against another tree, and then it was obvious. 
> But what if the empty tree is a subdirectory of another tree - how do you 
> express that in a diff? Do you care? Right now, since we always recurse 
> into the tree (and then not find anything), empty trees will simply not 
> show up _at_all_ in any diffs.
>
> And what about usability issues elsewhere? With my patch, doing something 
> like a
>
> 	git add directory/
>
> still won't do anything, because the behaviour of "git add" has always 
> been to recurse into directories. So to add a new empty directory, you'd 
> have to do
>
> 	git update-index --add directory
>
> and that's not exactly user-friendly.
>
> So do you add a "-n" flag to "git add" to tell it to not recurse? Or do 
> you always recurse, but then if you notice that the end result is empty, 
> you add it as a directory?

Another issue I thought about was what you would do in the step
3 in the following:

 1. David says "mkdir D; git add D"; you add S_IFDIR entry in
    the index at D;

 2. David says "date >D/F; git add D/F"; presumably you drop D
    from the index (to keep the index more backward compatible)
    and add S_IFREG entry at D/F.

 3. David says "git rm D/F".

Have we stopped keeping track of the "empty directory" at this
point?

^ permalink raw reply

* Re: [REVISED PATCH 2/6] Introduce commit notes
From: Junio C Hamano @ 2007-07-19  5:13 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Johannes Schindelin, Alberto Bertogli, git, Johan Herland
In-Reply-To: <alpine.LFD.0.999.0707181949490.27353@woody.linux-foundation.org>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> And yes, the "search for zero bytes" is not *guaranteed* to find any 
> beginning at all, if you have lots of short names, *and* lots of zero 
> bytes in the SHA1's. But while short names may be common, zero bytes in 
> SHA1's are not so much (since you should expect to see a very even 
> distribution of bytes, and as such most SHA1's by far should have no zero 
> bytes at all!)
>
> So if you're really really *really* unlucky, you might end up having to 
> fall back on the linear search. But it still works!
>
> Can anybody see anything wrong in my thinking above?

Another anchoring clue you seem not to be exploiting fully is
that the ASCII part must match "^[1-7][0-7]{4,5} " (mode bytes).
But the real problem of this approach of course is that this is
not reliable and can get a false match.  You can find your
beginning NUL in the SHA-1 part of one entry, and terminating
NUL later in the SHA-1 part of next entry, and you will never
notice.

However, in the case of Dscho's "notes" code, I do not think (1)
you do not have to guess like the above, and (2) the problem is
much simpler.

Dcsho's "note" looks like a tree full of two-byte [0-9a-f]{2}
names, each of them points at another tree, with the second
level tree being full of 32-byte [0-9a-f]{38} names, each of
them points at a blob.  So it is a much more regular, strict
shape.  And in order to look for a note for an object whose name
is ([0-9a-f]{2})([0-9a-f]{38}), you will find the blob that is
at "$1/$2" in a "note".

I was suggesting to have a specialized parser only to read such
tree objects that are "abused" to represent notes.  You can
cheaply validate that these trees are of expected shape.

 (1) Validate that size of the toplevel tree is multiple of 29 =
     (5 + 1 + 2 + 1 + 20); the second level should be multiple
     of 66 = (6 + 1 + 38 + 1 + 20).  These two levels of trees
     are of fixed-entry-length that allows easy binary search.

 (2) While binary searching trees of either level, you can
     validate that the entry looks like from a note (for the
     toplevel, "40000 [0-9a-f]{2}\0", for the second level,
     "100644 [0-9a-f]{38}\0").

For an added safety, a "notes" writer could even throw in
signature bytes (say, a symlink whose name is " !" in the
top-level tree, and another symlink " !{37}" in the second-level
tree) to protect the reader.

^ permalink raw reply

* [PATCH] Mak gitk bind keyboard actions to the command key on Mac OS
From: Shawn O. Pearce @ 2007-07-19  4:37 UTC (permalink / raw)
  To: Junio C Hamano, Paul Mackerras; +Cc: git

git-gui already uses the command key for accelerators, but gitk has
never done so.  I'm actually finding it very hard to move back and
forth between the two applications as git-gui is following the Mac
OS X conventions and gitk is not.

This trick is the same one that git-gui uses to determine which
key to bind actions to.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 gitk |   75 +++++++++++++++++++++++++++++++++++++----------------------------
 1 files changed, 43 insertions(+), 32 deletions(-)

diff --git a/gitk b/gitk
index 39e452a..6f58803 100755
--- a/gitk
+++ b/gitk
@@ -796,6 +796,12 @@ proc makewindow {} {
         wm geometry . "$geometry(main)"
     }
 
+    if {[tk windowingsystem] eq {aqua}} {
+        set M1B M1
+    } else {
+        set M1B Control
+    }
+
     bind .pwbottom <Configure> {resizecdetpanes %W %w}
     pack .ctop -fill both -expand 1
     bindall <1> {selcanvline %W %x %y}
@@ -814,12 +820,12 @@ proc makewindow {} {
     bindkey <Key-Left> "goback"
     bind . <Key-Prior> "selnextpage -1"
     bind . <Key-Next> "selnextpage 1"
-    bind . <Control-Home> "allcanvs yview moveto 0.0"
-    bind . <Control-End> "allcanvs yview moveto 1.0"
-    bind . <Control-Key-Up> "allcanvs yview scroll -1 units"
-    bind . <Control-Key-Down> "allcanvs yview scroll 1 units"
-    bind . <Control-Key-Prior> "allcanvs yview scroll -1 pages"
-    bind . <Control-Key-Next> "allcanvs yview scroll 1 pages"
+    bind . <$M1B-Home> "allcanvs yview moveto 0.0"
+    bind . <$M1B-End> "allcanvs yview moveto 1.0"
+    bind . <$M1B-Key-Up> "allcanvs yview scroll -1 units"
+    bind . <$M1B-Key-Down> "allcanvs yview scroll 1 units"
+    bind . <$M1B-Key-Prior> "allcanvs yview scroll -1 pages"
+    bind . <$M1B-Key-Next> "allcanvs yview scroll 1 pages"
     bindkey <Key-Delete> "$ctext yview scroll -1 pages"
     bindkey <Key-BackSpace> "$ctext yview scroll -1 pages"
     bindkey <Key-space> "$ctext yview scroll 1 pages"
@@ -839,15 +845,15 @@ proc makewindow {} {
     bindkey ? findprev
     bindkey f nextfile
     bindkey <F5> updatecommits
-    bind . <Control-q> doquit
-    bind . <Control-f> dofind
-    bind . <Control-g> {findnext 0}
-    bind . <Control-r> dosearchback
-    bind . <Control-s> dosearch
-    bind . <Control-equal> {incrfont 1}
-    bind . <Control-KP_Add> {incrfont 1}
-    bind . <Control-minus> {incrfont -1}
-    bind . <Control-KP_Subtract> {incrfont -1}
+    bind . <$M1B-q> doquit
+    bind . <$M1B-f> dofind
+    bind . <$M1B-g> {findnext 0}
+    bind . <$M1B-r> dosearchback
+    bind . <$M1B-s> dosearch
+    bind . <$M1B-equal> {incrfont 1}
+    bind . <$M1B-KP_Add> {incrfont 1}
+    bind . <$M1B-minus> {incrfont -1}
+    bind . <$M1B-KP_Subtract> {incrfont -1}
     wm protocol . WM_DELETE_WINDOW doquit
     bind . <Button-1> "click %W"
     bind $fstring <Key-Return> dofind
@@ -1088,12 +1094,17 @@ proc keys {} {
 	raise $w
 	return
     }
+    if {[tk windowingsystem] eq {aqua}} {
+	set M1T Cmd
+    } else {
+	set M1T Ctrl
+    }
     toplevel $w
     wm title $w "Gitk key bindings"
-    message $w.m -text {
+    message $w.m -text "
 Gitk key bindings:
 
-<Ctrl-Q>		Quit
+<$M1T-Q>		Quit
 <Home>		Move to first commit
 <End>		Move to last commit
 <Up>, p, i	Move up one commit
@@ -1102,12 +1113,12 @@ Gitk key bindings:
 <Right>, x, l	Go forward in history list
 <PageUp>	Move up one page in commit list
 <PageDown>	Move down one page in commit list
-<Ctrl-Home>	Scroll to top of commit list
-<Ctrl-End>	Scroll to bottom of commit list
-<Ctrl-Up>	Scroll commit list up one line
-<Ctrl-Down>	Scroll commit list down one line
-<Ctrl-PageUp>	Scroll commit list up one page
-<Ctrl-PageDown>	Scroll commit list down one page
+<$M1T-Home>	Scroll to top of commit list
+<$M1T-End>	Scroll to bottom of commit list
+<$M1T-Up>	Scroll commit list up one line
+<$M1T-Down>	Scroll commit list down one line
+<$M1T-PageUp>	Scroll commit list up one page
+<$M1T-PageDown>	Scroll commit list down one page
 <Shift-Up>	Move to previous highlighted line
 <Shift-Down>	Move to next highlighted line
 <Delete>, b	Scroll diff view up one page
@@ -1115,20 +1126,20 @@ Gitk key bindings:
 <Space>		Scroll diff view down one page
 u		Scroll diff view up 18 lines
 d		Scroll diff view down 18 lines
-<Ctrl-F>		Find
-<Ctrl-G>		Move to next find hit
+<$M1T-F>		Find
+<$M1T-G>		Move to next find hit
 <Return>	Move to next find hit
 /		Move to next find hit, or redo find
 ?		Move to previous find hit
 f		Scroll diff view to next file
-<Ctrl-S>		Search for next hit in diff view
-<Ctrl-R>		Search for previous hit in diff view
-<Ctrl-KP+>	Increase font size
-<Ctrl-plus>	Increase font size
-<Ctrl-KP->	Decrease font size
-<Ctrl-minus>	Decrease font size
+<$M1T-S>		Search for next hit in diff view
+<$M1T-R>		Search for previous hit in diff view
+<$M1T-KP+>	Increase font size
+<$M1T-plus>	Increase font size
+<$M1T-KP->	Decrease font size
+<$M1T-minus>	Decrease font size
 <F5>		Update
-} \
+" \
 	    -justify left -bg white -border 2 -relief groove
     pack $w.m -side top -fill both -padx 2 -pady 2
     $w.m configure -font $uifont
-- 
1.5.3.rc2.822.g75e72

^ permalink raw reply related


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