Git development
 help / color / mirror / Atom feed
* Re: [PATCH] gitk: Add shortcut Ctrl-W for closing the active window
From: Chris Frey @ 2010-02-02 23:51 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: Paul Mackerras, Git Mailing List
In-Reply-To: <4B68A310.6000807@web.de>

On Tue, Feb 02, 2010 at 11:11:28PM +0100, Jens Lehmann wrote:
> To make the user experience between git gui and gitk more homogeneous, use
> Ctrl-W in gitk too for closing the active window. When closing the main
> window doquit is called for proper cleanup.

This is sort of like when Mozilla claims Ctrl-U to show the page source,
and you're expecting it to clear the line.

At least in Mozilla, you can configure the browser to respect the
old meanings for Ctrl-U, etc.

- Chris

^ permalink raw reply

* Re: [BUG] - "git commit --amend" commits, when exiting the editor  with no changes written
From: Erik Faye-Lund @ 2010-02-03  0:16 UTC (permalink / raw)
  To: Eugene Sajine
  Cc: Johannes Schindelin, Wincent Colaiuta, Avery Pennarun,
	Jacob Helwig, git
In-Reply-To: <76c5b8581002021548n129b3997r48ee6f6df5a1a4eb@mail.gmail.com>

On Wed, Feb 3, 2010 at 12:48 AM, Eugene Sajine <euguess@gmail.com> wrote:
>>
>> This is where you're wrong. The reason why commit (without --amend) is
>> aborting when you don't save is that the default commit message is
>> empty, not that it wasn't saved. In fact, just saving still makes in
>> abort. You need to add something to the commit message and then save
>> to get it to perform the commit.
>>
>>> Same should be applied to rebase -i and of course to commit --amend as
>>> it is still commit operation.
>>>
>>
>> "rebase -i" and "commit --amend" already has the exact same logic as
>> commit without ammend in this regard - they abort if the buffer is
>> empty.
>>
>> --
>> Erik "kusma" Faye-Lund
>>
>
> Thank you, but I'm not wrong, as i described the exact same thing
> somewhere in my second or third message, without even knowing the
> implementation details. I understand the reason WHY it is like it is,
> i just don't like it as it is inconsistent and IMHO incorrect.
>

But you are. The interface isn't inconsistent. Your model of how it
works however, is wrong.

-- 
Erik "kusma" Faye-Lund

^ permalink raw reply

* Re: [BUG] - "git commit --amend" commits, when exiting the editor  with no changes written
From: Eugene Sajine @ 2010-02-03  0:09 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Wincent Colaiuta, Avery Pennarun, Jacob Helwig, git
In-Reply-To: <alpine.DEB.1.00.1002030056480.4985@pacific.mpi-cbg.de>

> Sorry, your "if message was not saved, abort" is so out of line with any
> program I use that this would be highly confusing (maybe "complicated" was
> the wrong choice of words).

Frankly, i think it is in line with "git commit" and that's the only
thing that matters here as I have no idea what programs you're talking
about. If your programs are working without making sure that user
confirmed whatever he actually supposed to confirm, then you probably
should revisit them;)

Thanks,
Eugene

^ permalink raw reply

* Re: [PATCH 1/2] gitweb: Add an option to force version match
From: Jakub Narebski @ 2010-02-02 23:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: John 'Warthog9' Hawley, git
In-Reply-To: <7vvdef1by2.fsf@alter.siamese.dyndns.org>

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

> "John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes:

> > @@ -583,6 +586,33 @@ sub get_loadavg {
> >  our $git_version = qx("$GIT" --version) =~ m/git version (.*)$/ ? $1 : "unknown";
> >  $number_of_git_cmds++;
> >  
> > +# Throw an error if git versions does not match, if $git_versions_must_match is true.
> > +if ($git_versions_must_match &&
> > +    $git_version ne $version) {
> > +	my $admin_contact =
> > +		defined $ENV{'SERVER_ADMIN'} ? ", $ENV{'SERVER_ADMIN'}," : '';
> > +	my $err_msg = <<EOT;
> > +<h1 align="center">*** Warning ***</h1>
> > +<p>
> > +This version of gitweb was compiled for <b>@{[esc_html($version)]}</b>,
> > +however git version <b>@{[esc_html($git_version)]}</b> was found on server.
> > +Running an instance of gitweb that is not matched to the git binaries may
> > +result in unexpected behavior of gitweb, and loss of functionality or
> > +incorrect data on displayed pages.
> > +</p>
> > +<p>
> > +Please update the git or gitweb installation so that their versions match, or
> > +if you feel you are sure that you wish to proceed with running gitweb
> > +with unmatched versions please contact the server administrator${admin_contact}
> > +to configure gitweb to allow mismatched versions.  This can be done by
> > +setting \$git_versions_must_match to @{[esc_html($git_versions_must_match)]}

Errr... the above line does not makes sense, as $git_versions_must_match
is true (true value), and not false (false value). so you would get something
like:

  'setting $git_versions_must_match to 1 (false value) [...]'

> > +(false value) in gitweb configuration file,
> > +'@{[esc_path(-e $GITWEB_CONFIG ? $GITWEB_CONFIG : $GITWEB_CONFIG_SYSTEM)]}'.
> > +</p>
> > +EOT
> > +	die_error(500, 'Internal server error', $err_msg);

As I wrote in my earlier response, this is not enough.  If this error
message (description of this situation) is meant to describe how to turn
this feature off, it has to deal with situation where config file does
not exist.  With this feature off by default we knew that it had to be
changed in some gitweb config file, so one of $GITWEB_CONFIG and
$GITWEB_CONFIG_SYSTEM must exists.  This is not true if this feature is
turned on by default.

You need to describe how to deal with the following situations:

* Both $GITWEB_CONFIG and $GITWEB_CONFIG_SYSTEM are empty or undefined.
  You might skip this situation, as by default both are set by
  gitweb/Makefile, $GITWEB_CONFIG to gitweb_config.perl and
  $GITWEB_CONFIG_SYSTEM to /etc/gitweb.conf
 
* Both $GITWEB_CONFIG and $GITWEB_CONFIG_SYSTEM are set and non-empty,
  but neither file exists (this means that $projects_list is set during
  build stage).

Current code deals correctly only with situation where either
$GITWEB_CONFIG or $GITWEB_CONFIG_SYSTEM exists, and it further assumes
that neither is undefined (you would get perl errors in case if either
is undefined).

> Why, why, why?
> 
> This is not even a "*** Warning ***".  You are refusing to let them do
> anything useful until they either flip the bit off or reinstall git and/or
> gitweb.  It is a _fatal error_ message.

Ooops.

> 
> To whom are you giving this _warning_?  Please read the message yourself
> again.

The original message (from "Gitweb caching v2" thread) was ment purely
for server administrator.  Current version tries to address both
ordinary user (which has to contact gitweb administrator) and gitweb
administrator (who needs to know how to remote error condition, either
by bringing git and gitweb versions in sync, or by changing
configuration).  And does it badly...

[...]
> So I have to ask a basic question I asked (at least I tried to) last night
> again.  Whom are you trying to help?
[...]

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH 2/2] gitweb: Fix chop_str to allow for & characters in strings
From: J.H. @ 2010-02-02 23:54 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <m3mxzr9pcz.fsf@localhost.localdomain>

On 02/02/2010 03:43 PM, Jakub Narebski wrote:
> "John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes:
> 
>> I'm unsure why this was placed in their originally, and it seems to
>> ultimately stem from code from before gitweb was merged into git core,
>> but there's an instance where git chops a string incorrectly based on
>> this.
>>
>> Specifically:
>>
>> 	API & protocol: support option to force written data immediately to disk
>>
>> from http://git.kernel.org/?p=daemon/distsrv/chunkd.git;a=commit;h=3b02f749df2cb1288f345a689d85e7061f507e54
>>
>> The short version of the title gets chopped to
>>
>> 	API ...
>>
>> where it should be
>>
>> 	API & protocol: support option to force written data...
>>
>> This reverts that specific problem.
>> ---
>>  gitweb/gitweb.perl |    1 -
>>  1 files changed, 0 insertions(+), 1 deletions(-)
>>
>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index 57771a0..4cc6d19 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -1382,7 +1382,6 @@ sub chop_str {
>>  		my $body = $1;
>>  		my $tail = $2;
>>  		if (length($tail) > 4) {
>> -			$body =~ s/&[^;]*$//;
>>  			$tail = "... ";
>>  		}
>>  		return "$body$tail";
> 
> I think it is a good change.  chop_str is meant to be used _before_
> HTML escaping (esc_html or equivalent) is to be applied; removed line
> looks like it was meant (badly) to always remove HTML entities
> fully... but those entities are only added later.
> 
> So now what is left is to come up with proper commit message, and add
> 
> Signed-off-by: John 'Warthog9' Hawley" <warthog9@kernel.org>
> Acked-by: Jakub Narebski <jnareb@gmail.com>
> 

There's a couple more lines, similar to that, ins chop_str.  The bug I
needed to fix didn't tickle those, so I made the smallest change
possible.  But those should probably be looked at as well.

- John 'Warthog9' Hawley

^ permalink raw reply

* Re: [BUG] - "git commit --amend" commits, when exiting the editor  with no changes written
From: Johannes Schindelin @ 2010-02-02 23:58 UTC (permalink / raw)
  To: Eugene Sajine; +Cc: Wincent Colaiuta, Avery Pennarun, Jacob Helwig, git
In-Reply-To: <76c5b8581002021534i1f0dbd18y1b216233d282cd84@mail.gmail.com>

Hi,

On Tue, 2 Feb 2010, Eugene Sajine wrote:

> > Of course, you could come up with wonderfully complicated strategies 
> > such as "if nothing was added, then an unmodified commit message means 
> > 'abort', else it means 'go ahead'", but I hope that you agree that 
> > this would be very bad from the users' POV: it would be utterly 
> > confusing.
> 
> No complicated strategies necessary.

Sorry, your "if message was not saved, abort" is so out of line with any 
program I use that this would be highly confusing (maybe "complicated" was 
the wrong choice of words).

IMO it would not buy us anything over the current solution, which also 
requires the user to read the manual.

Ciao,
Dscho

^ permalink raw reply

* Re: [BUG] - "git commit --amend" commits, when exiting the editor  with no changes written
From: Eugene Sajine @ 2010-02-02 23:48 UTC (permalink / raw)
  To: kusmabite
  Cc: Johannes Schindelin, Wincent Colaiuta, Avery Pennarun,
	Jacob Helwig, git
In-Reply-To: <40aa078e1002021540i13e35776xd733a4e40f34b477@mail.gmail.com>

>
> This is where you're wrong. The reason why commit (without --amend) is
> aborting when you don't save is that the default commit message is
> empty, not that it wasn't saved. In fact, just saving still makes in
> abort. You need to add something to the commit message and then save
> to get it to perform the commit.
>
>> Same should be applied to rebase -i and of course to commit --amend as
>> it is still commit operation.
>>
>
> "rebase -i" and "commit --amend" already has the exact same logic as
> commit without ammend in this regard - they abort if the buffer is
> empty.
>
> --
> Erik "kusma" Faye-Lund
>

Thank you, but I'm not wrong, as i described the exact same thing
somewhere in my second or third message, without even knowing the
implementation details. I understand the reason WHY it is like it is,
i just don't like it as it is inconsistent and IMHO incorrect.

Thanks,
Eugene

^ permalink raw reply

* Re: Implement --password option for git svn perl script
From: Frank Li @ 2010-02-02 23:47 UTC (permalink / raw)
  To: Laszlo Papp; +Cc: Eric Wong, Tay Ray Chuan, git
In-Reply-To: <a362e8011002020516i5d5adcf7k6553d62e633cec88@mail.gmail.com>

Is it possible use OpenSSH method to let GUI to input passwd.
environment SSH_ASKPASS, which point to a application. when
passwd need, openssh launch this GUI application,  user can input
passwd at this GUI application.

^ permalink raw reply

* Re: [BUG] - "git commit --amend" commits, when exiting the editor  with no changes written
From: Junio C Hamano @ 2010-02-02 23:44 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Eugene Sajine, Wincent Colaiuta, Avery Pennarun, Jacob Helwig,
	git
In-Reply-To: <alpine.DEB.1.00.1002022356460.4985@pacific.mpi-cbg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> No. If not saving, then it means that the user was fine with the message 
> as-is.
>
> Remember, --amend is usually for amending the _content_ of the patch, not 
> so much the message.

I very often use --amend to update only the message, so I would strongly
disagree with the "usually".

But that doesn't change my conclusion ;-) It is immaterial if you amended
the tree or the message.  If either or both changed, we do need to create
a new commit---there is no question about it, so which one is more often
done does not matter.  Eugene does not gain anything by "Remember"ing it.

What is more important is what to do when neither changed.

If you run "rebase -i" and leave the earlier "pick"s intact, it seems to
try keeping the commits intact, instead of rewriting.

It is understandable if somebody wanted to keep using the old commit
object when the committer timestamp is the _only_ thing that is different
between the original and the amended commit, after "git commit --amend" is
run.  Essentially, the behaviour of "rebase -i" for earlier "pick"s comes
from the same motivation.

Right now, if you run this in a loop:

    while hell has not frozen over
    do
	git reset && EDITOR=: git commit --amend
    done

it will keep generating almost identical commits, except for their
committer timestamps.  The very first one may have different committer
identity from the original if you are amending somebody else's commit, but
after that it will be all you over and over again, only at different
times.

I am inclined to say that it is a feature that it updates the commit in
such a case to record the _last time_ you touched the commit.  You ran
"commit --amend" and did not use the any published method to _abort_ the
amend.

As mentioned already, @{1} is your friend.  Regardless of the difference
between @{0} and @{1}, it is a way to go back to the previous commit if
you do not like the latest commit for whatever reason; the latest commit
may have a "wrong tree", a "wrong message", or a "wrong committer
timestamp".

^ permalink raw reply

* Re: [PATCH 2/2] gitweb: Fix chop_str to allow for & characters in strings
From: Jakub Narebski @ 2010-02-02 23:43 UTC (permalink / raw)
  To: John 'Warthog9' Hawley; +Cc: git
In-Reply-To: <1265147814-13284-3-git-send-email-warthog9@eaglescrag.net>

"John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes:

> I'm unsure why this was placed in their originally, and it seems to
> ultimately stem from code from before gitweb was merged into git core,
> but there's an instance where git chops a string incorrectly based on
> this.
> 
> Specifically:
> 
> 	API & protocol: support option to force written data immediately to disk
> 
> from http://git.kernel.org/?p=daemon/distsrv/chunkd.git;a=commit;h=3b02f749df2cb1288f345a689d85e7061f507e54
> 
> The short version of the title gets chopped to
> 
> 	API ...
> 
> where it should be
> 
> 	API & protocol: support option to force written data...
> 
> This reverts that specific problem.
> ---
>  gitweb/gitweb.perl |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 57771a0..4cc6d19 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1382,7 +1382,6 @@ sub chop_str {
>  		my $body = $1;
>  		my $tail = $2;
>  		if (length($tail) > 4) {
> -			$body =~ s/&[^;]*$//;
>  			$tail = "... ";
>  		}
>  		return "$body$tail";

I think it is a good change.  chop_str is meant to be used _before_
HTML escaping (esc_html or equivalent) is to be applied; removed line
looks like it was meant (badly) to always remove HTML entities
fully... but those entities are only added later.

So now what is left is to come up with proper commit message, and add

Signed-off-by: John 'Warthog9' Hawley" <warthog9@kernel.org>
Acked-by: Jakub Narebski <jnareb@gmail.com>

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [BUG] - "git commit --amend" commits, when exiting the editor  with no changes written
From: Avery Pennarun @ 2010-02-02 23:40 UTC (permalink / raw)
  To: Eugene Sajine; +Cc: Wincent Colaiuta, Jacob Helwig, git
In-Reply-To: <76c5b8581002021502i2bb34967y9a88d8b25ce7fa42@mail.gmail.com>

On Tue, Feb 2, 2010 at 6:02 PM, Eugene Sajine <euguess@gmail.com> wrote:
> On Tue, Feb 2, 2010 at 5:35 PM, Avery Pennarun <apenwarr@gmail.com> wrote:
>>This feature would have to be
>> optional in order to not confuse existing users, and not annoy users
>> of editors (like my favourite, joe) which don't save-on-exit if the
>> file hasn't changed.  But I think it might be valuable to some people
>> nevertheless.  And if it became popular, perhaps it could become the
>> default in some future version of git (after giving people enough
>> notice, etc, etc).
>
> But here I totally disagree, because i don't understand what is so new
> and confusing in the workflow I'm talking about?
> Why you're not confused that you MUST save before exiting your EDITOR
> in order to be able to commit (else it will fail), but the same
> workflow suddenly becomes confusing when you doing "commit --amend" or
> especially "rebase -i" ???

I think you're missing the point of people's objections.

It's pretty clear that the current behaviour is inconsistent and
confusing to new users:

- If you make a new commit and change your mind, abort your editor:
commit is aborted.

- If you amend a commit and change your mind, abort your editor:
commit gets *replaced* and newbies don't know how to get it back.
Argh!

However, whether it's inconsistent and confusing is unfortunately not
the question.  The question is what can you do to possibly improve
things while a) not making backwards-incompatible changes, b) not
making things even more inconsistent, and c) not annoying existing
long-term users who like it fine the way it is.  (I use the current
behaviour and I even kind of like it now that I understand it.  But I
still think it's confusing and admit there may be a better way.)

git development is kind of like working at Intel or Microsoft.  If you
want to achieve world domination, you don't make new versions that are
incompatible with old ones, no matter how stupid you think the old one
was.

You can however add *new* stuff.  That's why I suggested adding an
option.  You could even make it a config option so you only have to
set it once (just like setting your preferred editor).

> I wish i could - but, unfortunately, I'm as far from C as from the Sun
> (star;)). I'm developing a little bit in Java, but can't do C.

There's no better time to learn :)

Have fun,

Avery

^ permalink raw reply

* Re: [BUG] - "git commit --amend" commits, when exiting the editor  with no changes written
From: Erik Faye-Lund @ 2010-02-02 23:40 UTC (permalink / raw)
  To: Eugene Sajine
  Cc: Johannes Schindelin, Wincent Colaiuta, Avery Pennarun,
	Jacob Helwig, git
In-Reply-To: <76c5b8581002021534i1f0dbd18y1b216233d282cd84@mail.gmail.com>

On Wed, Feb 3, 2010 at 12:34 AM, Eugene Sajine <euguess@gmail.com> wrote:
>> Of course, you could come up with wonderfully complicated strategies such
>> as "if nothing was added, then an unmodified commit message means 'abort',
>> else it means 'go ahead'", but I hope that you agree that this would be
>> very bad from the users' POV: it would be utterly confusing.
>
> No complicated strategies necessary.
> I'm not talking about modified or not modified commit message. I'm not
> forcing anybody to modify commit message in order to proceed with
> commit --amend.
> I'm talking about consistent behaviour of main operations:
>
> Commit - you must save on exit from your editor, simply exiting will abort.
>

This is where you're wrong. The reason why commit (without --amend) is
aborting when you don't save is that the default commit message is
empty, not that it wasn't saved. In fact, just saving still makes in
abort. You need to add something to the commit message and then save
to get it to perform the commit.

> Same should be applied to rebase -i and of course to commit --amend as
> it is still commit operation.
>

"rebase -i" and "commit --amend" already has the exact same logic as
commit without ammend in this regard - they abort if the buffer is
empty.

-- 
Erik "kusma" Faye-Lund

^ permalink raw reply

* Re: [BUG] - "git commit --amend" commits, when exiting the editor  with no changes written
From: Eugene Sajine @ 2010-02-02 23:34 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Wincent Colaiuta, Avery Pennarun, Jacob Helwig, git
In-Reply-To: <alpine.DEB.1.00.1002022356460.4985@pacific.mpi-cbg.de>

> Of course, you could come up with wonderfully complicated strategies such
> as "if nothing was added, then an unmodified commit message means 'abort',
> else it means 'go ahead'", but I hope that you agree that this would be
> very bad from the users' POV: it would be utterly confusing.

No complicated strategies necessary.
I'm not talking about modified or not modified commit message. I'm not
forcing anybody to modify commit message in order to proceed with
commit --amend.
I'm talking about consistent behaviour of main operations:

Commit - you must save on exit from your editor, simply exiting will abort.

Same should be applied to rebase -i and of course to commit --amend as
it is still commit operation.

Even if the text is OK or you agree with generated sequence of rebase
- confirm that explicitly by saving on exit from the editor, else it
should abort. Pretty obvious to me...;)


Thanks,
'Eugene

^ permalink raw reply

* Re: [BUG] - "git commit --amend" commits, when exiting the editor  with no changes written
From: Eugene Sajine @ 2010-02-02 23:27 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Avery Pennarun, Wincent Colaiuta, Jacob Helwig, git
In-Reply-To: <alpine.DEB.1.00.1002030014100.4985@pacific.mpi-cbg.de>

>> Frankly, I think that "rebase -i" is the best example when exiting
>> without save should abort, not proceed.
>
> That would be horrible! If the list of commits I want to apply happens to
> be exactly what I want without needing any reordering/removing, then it
> would abort, according to you.
>

No! It wouldn't if you save instead of simply exiting, if you type :wq
instead :q

I'm obviously failing to express my self ...;)

NOTHING is supposed to change for you except that you have to save on
exit, write the file, confirm changes or absence of such, agree with
generated content of rebase sequence. That is all i'm talking about.

You MUST save the file when you do commit - right? Because this file
COMMIT_EDITMSG is empty. It will fail to commit otherwise - right?
The problem is that this file is NOT empty when there is a
prepopulated message like rebase -i sequence of commits or message
from commit being amended.
Therefore, you can exit without saving and the action will still
happen - which is wrong and not consistent with "commit"!

Exit without saving for me means FORGET what i was doing here . So, it
is obvious, that when I'm doing rebase -i and working with the list of
commits exit without saving should mean - FORGET about rebase.

Thanks,
Eugene

^ permalink raw reply

* Re: [BUG] - "git commit --amend" commits, when exiting the editor  with no changes written
From: Johannes Schindelin @ 2010-02-02 23:15 UTC (permalink / raw)
  To: Eugene Sajine; +Cc: Avery Pennarun, Wincent Colaiuta, Jacob Helwig, git
In-Reply-To: <76c5b8581002021502i2bb34967y9a88d8b25ce7fa42@mail.gmail.com>

Hi,

On Tue, 2 Feb 2010, Eugene Sajine wrote:

> Frankly, I think that "rebase -i" is the best example when exiting 
> without save should abort, not proceed.

That would be horrible! If the list of commits I want to apply happens to 
be exactly what I want without needing any reordering/removing, then it 
would abort, according to you.

Ciao,
Dscho

^ permalink raw reply

* Re: [BUG] - "git commit --amend" commits, when exiting the editor  with no changes written
From: Eugene Sajine @ 2010-02-02 23:02 UTC (permalink / raw)
  To: Avery Pennarun; +Cc: Wincent Colaiuta, Jacob Helwig, git
In-Reply-To: <32541b131002021435kadb68ffge77ad5f4e1775418@mail.gmail.com>

On Tue, Feb 2, 2010 at 5:35 PM, Avery Pennarun <apenwarr@gmail.com> wrote:
>> amend or rebase:
>> COMMIT_EDITMSG = "message from commit or rebase sequence";
>> $ git commit --amend
>>
>> in background:
>> $EDITOR .git/COMMIT_EDITMSG
>>
>> COMMIT_EDITMSG = NULL    <======== this i think is missing. We have to
>> clean it after it is populated in the editor.
>
> That's not going to work.  Most editors will get quite angry at you if
> you modify the file while the editor has it open.  Furthermore, how do
> you know how long to wait after launching the editor but before
> deleting the file?

I'm not insisting on this approach by any mean - i was trying to
explain what is the problem i consider to be a bug.


>
> Like I suggested earlier, monitoring the mtime of the file to see if
> it has changed after the editor exits, even if the file itself is
> identical to what it was, might work.

Anything that would work;)

>This feature would have to be
> optional in order to not confuse existing users, and not annoy users
> of editors (like my favourite, joe) which don't save-on-exit if the
> file hasn't changed.  But I think it might be valuable to some people
> nevertheless.  And if it became popular, perhaps it could become the
> default in some future version of git (after giving people enough
> notice, etc, etc).

But here I totally disagree, because i don't understand what is so new
and confusing in the workflow I'm talking about?
Why you're not confused that you MUST save before exiting your EDITOR
in order to be able to commit (else it will fail), but the same
workflow suddenly becomes confusing when you doing "commit --amend" or
especially "rebase -i" ???

Frankly, I think that "rebase -i" is the best example when exiting
without save should abort, not proceed.


>
> So you might want to submit a patch to implement an option like that.
>
> Avery
>

I wish i could - but, unfortunately, I'm as far from C as from the Sun
(star;)). I'm developing a little bit in Java, but can't do C.

Thanks,
Eugene

^ permalink raw reply

* Re: [PATCH 1/2] gitweb: Add an option to force version match
From: Junio C Hamano @ 2010-02-02 22:59 UTC (permalink / raw)
  To: John 'Warthog9' Hawley; +Cc: git
In-Reply-To: <1265147814-13284-2-git-send-email-warthog9@eaglescrag.net>

"John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes:

John, I'm sorry but I have to say this is somewhat incoherent.

> + * $git_versions_must_match
> +   If set to true value, gitweb fails with "500 Internal Server Error" error
> +   if the version of the gitweb doesn't match version of the git binary.
> +   Gitweb can usually run with a mismatched git install.   The default is 1
> +   (true).

I would understand that it if this were "Gitweb seldom runs correctly with
unmatched version of git, so this defaults to true".  If it can _usually_
run just fine, why should everybody need to flip this off?  This doesn't
make any sense to me.

> @@ -583,6 +586,33 @@ sub get_loadavg {
>  our $git_version = qx("$GIT" --version) =~ m/git version (.*)$/ ? $1 : "unknown";
>  $number_of_git_cmds++;
>  
> +# Throw an error if git versions does not match, if $git_versions_must_match is true.
> +if ($git_versions_must_match &&
> +    $git_version ne $version) {
> +	my $admin_contact =
> +		defined $ENV{'SERVER_ADMIN'} ? ", $ENV{'SERVER_ADMIN'}," : '';
> +	my $err_msg = <<EOT;
> +<h1 align="center">*** Warning ***</h1>
> +<p>
> +This version of gitweb was compiled for <b>@{[esc_html($version)]}</b>,
> +however git version <b>@{[esc_html($git_version)]}</b> was found on server.
> +Running an instance of gitweb that is not matched to the git binaries may
> +result in unexpected behavior of gitweb, and loss of functionality or
> +incorrect data on displayed pages.
> +</p>
> +<p>
> +Please update the git or gitweb installation so that their versions match, or
> +if you feel you are sure that you wish to proceed with running gitweb
> +with unmatched versions please contact the server administrator${admin_contact}
> +to configure gitweb to allow mismatched versions.  This can be done by
> +setting \$git_versions_must_match to @{[esc_html($git_versions_must_match)]}
> +(false value) in gitweb configuration file,
> +'@{[esc_path(-e $GITWEB_CONFIG ? $GITWEB_CONFIG : $GITWEB_CONFIG_SYSTEM)]}'.
> +</p>
> +EOT
> +	die_error(500, 'Internal server error', $err_msg);

Why, why, why?

This is not even a "*** Warning ***".  You are refusing to let them do
anything useful until they either flip the bit off or reinstall git and/or
gitweb.  It is a _fatal error_ message.

To whom are you giving this _warning_?  Please read the message yourself
again.

The message tells _you_ to consider using matching versions (as if _you_
have the choice and authority to do so), hints _you_ to decide if you are
Ok with running an unmatched combination (again, as if _you_ have the
authority to make that decision), and then instructs _you_ to contact the
server administrator (who presumably can flip the bit).

That doesn't make _any_ sense to me.  Hopefully anybody who installs or
upgrades gitweb/git will hit his gitweb installation at least once before
end users start hitting, so I would understand it if you wrote the above
message addressed to the server administrator.

If somebody updates his git without bothering to update gitweb, on the
other hand, the end user may see the message before the administrator
does.  If git and gitweb might be managed by different people at a
particular site (k.org?), I would understand that the administrator of the
gitweb side _might_ want to be told about it by the end user, and the
above might be an attempt to make that happen.

But even in that case, out of the three instructions, only the last one is
for the end user, and telling him to be certain the combinations do work
before bugging the gitweb administrator doesn't make much sense to me.

So I have to ask a basic question I asked (at least I tried to) last night
again.  Whom are you trying to help?

Even if it is to help a gitweb administrator who is not in charge of other
people in the administrator group who would install unmatching versions of
git without telling him, would this really be the best solution?  You'd be
the first to suffer from this when HPA or whoever installs a new version
of git at k.org.  There should be a better way to help communication
between the people in the administration group, without involving or
inconveniencing the end users like this patch seems to do.

^ permalink raw reply

* Re: [BUG] - "git commit --amend" commits, when exiting the editor  with no changes written
From: Johannes Schindelin @ 2010-02-02 23:00 UTC (permalink / raw)
  To: Eugene Sajine; +Cc: Wincent Colaiuta, Avery Pennarun, Jacob Helwig, git
In-Reply-To: <76c5b8581002021431o2b5073a0s42b273b6d61893db@mail.gmail.com>

Hi,

On Tue, 2 Feb 2010, Eugene Sajine wrote:

[somebody wrote this, I suspect it was Wincent]
>
> > Why should Git care about how you exited from your EDITOR? All it 
> > should care about is the contents of the commit message, and the exit 
> > code of the editor.
> 
> It doesn't matter which editor is used. The only thing that matters is 
> that you saving or not saving on exit from editor. If not saving - abort 
> the action.

No. If not saving, then it means that the user was fine with the message 
as-is.

Remember, --amend is usually for amending the _content_ of the patch, not 
so much the message.

Of course, you could come up with wonderfully complicated strategies such 
as "if nothing was added, then an unmodified commit message means 'abort', 
else it means 'go ahead'", but I hope that you agree that this would be 
very bad from the users' POV: it would be utterly confusing.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] gitk: Add shortcut Ctrl-W for closing the active window
From: Jacob Todd @ 2010-02-02 17:50 UTC (permalink / raw)
  To: git
In-Reply-To: <4B68A310.6000807@web.de>

[-- Attachment #1: Type: text/plain, Size: 108 bytes --]

Last time I check ^w deleted the word before the cursor.

-- 
I am a man who does not exist for others.

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: [BUG] - "git commit --amend" commits, when exiting the editor  with no changes written
From: Avery Pennarun @ 2010-02-02 22:35 UTC (permalink / raw)
  To: Eugene Sajine; +Cc: Wincent Colaiuta, Jacob Helwig, git
In-Reply-To: <76c5b8581002021431o2b5073a0s42b273b6d61893db@mail.gmail.com>

> amend or rebase:
> COMMIT_EDITMSG = "message from commit or rebase sequence";
> $ git commit --amend
>
> in background:
> $EDITOR .git/COMMIT_EDITMSG
>
> COMMIT_EDITMSG = NULL    <======== this i think is missing. We have to
> clean it after it is populated in the editor.

That's not going to work.  Most editors will get quite angry at you if
you modify the file while the editor has it open.  Furthermore, how do
you know how long to wait after launching the editor but before
deleting the file?

Like I suggested earlier, monitoring the mtime of the file to see if
it has changed after the editor exits, even if the file itself is
identical to what it was, might work.  This feature would have to be
optional in order to not confuse existing users, and not annoy users
of editors (like my favourite, joe) which don't save-on-exit if the
file hasn't changed.  But I think it might be valuable to some people
nevertheless.  And if it became popular, perhaps it could become the
default in some future version of git (after giving people enough
notice, etc, etc).

So you might want to submit a patch to implement an option like that.

Avery

^ permalink raw reply

* Re: Fix signal handler
From: Jeff King @ 2010-02-02 22:32 UTC (permalink / raw)
  To: Markus Elfring; +Cc: git
In-Reply-To: <4B689CC5.3000400@web.de>

On Tue, Feb 02, 2010 at 10:44:37PM +0100, Markus Elfring wrote:

> > No, it's not a sig_atomic_t, but it is assignment of a single function
> > pointer that is properly declared as volatile. Is this actually a
> > problem on any known system?
> 
> Is it guaranteed to work on all supported software environments that an
> address can be atomically set?

I think you are missing my point. We are not coding to a set of
standards that provide guarantees. We are coding to a practical set of
real-world implementations that people try to run git on and produce bug
reports for. I do not think anyone on this list could even enumerate a
complete a list of the "supported software environments" of git.

We try to be conservative about portability issues. Some things are
obviously wrong. But other things may violate the letter of some
standards, and yet work in practice on all of the platforms people are
interested in running git on.

I don't think anyone here is much interested in whether there is any
sort of guarantee on a particular construct working. What we do care
about is whether there is an actual problem on some platform that enough
people care about to justify rewriting the code to handle it.

So to answer your question, I honestly don't know. The code may well be
broken on common platforms and it is simply a race condition that has
never come up. But I do know that it has not been a common source of bug
reports, which makes me not want to spend time investigating it when
nobody has demonstrated its incorrectness beyond mentioning a standards
document.  Especially when that time could be better spent fixing other
bugs.

-Peff

^ permalink raw reply

* Re: [BUG] - "git commit --amend" commits, when exiting the editor  with no changes written
From: Eugene Sajine @ 2010-02-02 22:31 UTC (permalink / raw)
  To: Wincent Colaiuta; +Cc: Avery Pennarun, Jacob Helwig, git
In-Reply-To: <77153A83-158D-4D36-A622-7AA3947C7D52@wincent.com>

>
> Why should Git care about how you exited from your EDITOR? All it should
> care about is the contents of the commit message, and the exit code of the
> editor.

It doesn't matter which editor is used. The only thing that matters is
that you saving or not saving on exit from editor. If not saving -
abort the action.

>
>> For example IF there is a variable somewhere where message is stored:
>> For new commit message it is NULL and when commit is ready it is set
>> up to "message". We check If it is not empty and not NULL (:wq
>> executed),  then we can commit.
>> For "prepopulated" messages this variable can be is set to "message"
>> from the beginning, so it doesn't matter if you're writing changes or
>> not the check will always be true, so we can commit.
>
> I don't understand what change you're asking for, as things already work as
> you've just described (the "variable" is the file, .git/COMMIT_EDITMSG).

So, i was right in my assumption about how it is implemented - good;)

>
>> If the implementation would be like this, then I would propose to
>> clean up this variable after the prepopulated message is populated, so
>> it won't use it blindly, but only after you're overwriting the message
>> (even if the message is not changed).
>
> Here is where you've lost me. Not sure what you mean here, nor how you're
> going to maintain the existing behavior for all of the people who have grown
> used to it over a long, long period of time.

Let me explain it like this (sorry for syntax):

new commit:

COMMIT_EDITMSG = NULL;
$ git commit

in background:
$EDITOR .git/COMMIT_EDITMSG

If saved/written
COMMIT_EDITMSG=message

if (COMMIT_EDITMSG != NULL &&  !COMMIT_EDITMSG.equalsIgnoreCase("")  )  {
commit();
} else abort();

====================
amend or rebase:
COMMIT_EDITMSG = "message from commit or rebase sequence";
$ git commit --amend

in background:
$EDITOR .git/COMMIT_EDITMSG

COMMIT_EDITMSG = NULL    <======== this i think is missing. We have to
clean it after it is populated in the editor.

If written back by save:
COMMIT_EDITMSG="message from commit or rebase sequence" (it was not
changed, but saved/written)

If not saved, the check below will abort the action

if (COMMIT_EDITMSG != NULL &&  !COMMIT_EDITMSG.equalsIgnoreCase("")  )  {
commit();
} else abort();


Is that a better explanation?

So, the only change from user perspective will be that you will have
always save/write your changes/rebase sequences (even if you don't
change the message) for the program to proceed with commit
amend/rebase or whatever the same way as for the simple commit.

Thanks,
Eugene

^ permalink raw reply

* Re: [BUG] - "git commit --amend" commits, when exiting the editor  with no changes written
From: Wincent Colaiuta @ 2010-02-02 22:06 UTC (permalink / raw)
  To: Eugene Sajine; +Cc: Avery Pennarun, Jacob Helwig, git
In-Reply-To: <76c5b8581002021356m52bb1817k9a4a29da0d4b681d@mail.gmail.com>

El 02/02/2010, a las 22:56, Eugene Sajine escribió:

>>
>> As has already been pointed out, an extremely common workflow for  
>> "git
>> commit --amend" is to include additional changes that were  
>> accidentally
>> overlooked  in the initial commit, and no changes are made to the  
>> commit
>> message.
>>
>> Changing the behavior would break that perfectly reasonable,
>> widely-practised workflow, and _that_ would be 100% not ok.
>
> I'm not against that workflow - I'm not forcing anybody to change the
> commit message. The thing I'm talking about is roughly that it should
> NOT treat the result of command ":q" the same way as the command
> ":wq".

Why should Git care about how you exited from your EDITOR? All it  
should care about is the contents of the commit message, and the exit  
code of the editor.

> For example IF there is a variable somewhere where message is stored:
> For new commit message it is NULL and when commit is ready it is set
> up to "message". We check If it is not empty and not NULL (:wq
> executed),  then we can commit.
> For "prepopulated" messages this variable can be is set to "message"
> from the beginning, so it doesn't matter if you're writing changes or
> not the check will always be true, so we can commit.

I don't understand what change you're asking for, as things already  
work as you've just described (the "variable" is the file, .git/ 
COMMIT_EDITMSG).

> If the implementation would be like this, then I would propose to
> clean up this variable after the prepopulated message is populated, so
> it won't use it blindly, but only after you're overwriting the message
> (even if the message is not changed).

Here is where you've lost me. Not sure what you mean here, nor how  
you're going to maintain the existing behavior for all of the people  
who have grown used to it over a long, long period of time.

Wincent

^ permalink raw reply

* Reacting to new commits in a poll-mirror
From: martin f krafft @ 2010-02-02 22:02 UTC (permalink / raw)
  To: git discussion list

[-- Attachment #1: Type: text/plain, Size: 827 bytes --]

Folks,

I am trying to set up Patchwork-Git integration on host A for
a project whose Git repo is on host B. I thought I could use
a mirror (git clone --mirror) on A and regularly fetch from B, but
I cannot find a way (hook) to make Git on A react to new commits it
fetched from B.

I can probably hack something up, e.g. store refs before a fetch and
then iterate the refs between the stored refs and the HEAD, but this
seems exceedingly hackish and I'd much rather have this information
from git-fetch (like git-push calls post-receive).

Is this at all possible?
Would it be possible?
Where would this be done?

-- 
martin | http://madduck.net/ | http://two.sentenc.es/
 
the uncertainty principle:
  you can never be sure how many
  beers you had last night.
 
spamtraps: madduck.bogus@madduck.net

[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* [PATCH] gitk: Add shortcut Ctrl-W for closing the active window
From: Jens Lehmann @ 2010-02-02 22:11 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: Git Mailing List

To make the user experience between git gui and gitk more homogeneous, use
Ctrl-W in gitk too for closing the active window. When closing the main
window doquit is called for proper cleanup.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

Found myself hitting Ctrl-W in gitk with no effect once too often ;-)

 gitk-git/gitk |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 03dd0f4..9629394 100644
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -2383,6 +2383,8 @@ proc makewindow {} {
     }
     bindall <$::BM> "canvscan mark %W %x %y"
     bindall <B$::BM-Motion> "canvscan dragto %W %x %y"
+    bind all <$M1B-Key-w> {destroy [winfo toplevel %W]}
+    bind . <$M1B-Key-w> doquit
     bindkey <Home> selfirstline
     bindkey <End> sellastline
     bind . <Key-Up> "selnextline -1"
@@ -2814,6 +2816,7 @@ proc keys {} {
 [mc "Gitk key bindings:"]

 [mc "<%s-Q>		Quit" $M1T]
+[mc "<%s-W>		Close window" $M1T]
 [mc "<Home>		Move to first commit"]
 [mc "<End>		Move to last commit"]
 [mc "<Up>, p, i	Move up one commit"]
-- 
1.7.0.rc1.173.gc7c5.dirty

^ 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