* Re: [PATCH (GITK) 5/6] gitk: Fixed automatic row selection during load.
From: Alexander Gavrilov @ 2008-07-31 12:41 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git
In-Reply-To: <18577.41259.758690.635991@cargo.ozlabs.ibm.com>
On Thu, Jul 31, 2008 at 3:25 PM, Paul Mackerras <paulus@samba.org> wrote:
> Alexander Gavrilov writes:
>
>> - Switching views now actually preserves the selected commit.
>> - Reloading (also Edit View) preserves the currently selected commit.
>> - Initial selection does not produce weird scrolling.
>>
>> Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
>
> I need a more detailed explanation of the rationale for the specific
> changes you have made in the changelog.
The rationale is that preserving the currently selected commit is more
intelligent behavior than always resetting to a preset position, and
it makes the UI feel more smooth. Also, although it is possible to
restore selection by clicking the 'back' button, it is not immediately
obvious; in fact I realized it only after reading the code.
Gitk already tried to preserve the commit in many cases, but failed
because of a conflicting change that made it select the head. Such
behavior is clearly a bug.
The Reload case is arguable, but I think that Edit View should
preserve the selection, and it uses Reload internally. It can be
resolved by adding a parameter to the function implementing Reload.
> As for the patch, it mostly looks good, but I have a few comments
> below.
>
>> +proc getcommits {selid} {
>> global canv curview need_redisplay viewactive
>>
>> initlayout
>> if {[start_rev_list $curview]} {
>> + reset_pending_select $selid
>
> Is there any significance to having the call to reset_pending_select
> after the start_rev_list call (other than not setting pending_select
> if start_rev_list fails)? I couldn't see any. If there is then it
> should be noted in a comment and/or the patch description.
It simply tries to preserve the original behavior.
>> @@ -503,7 +511,7 @@ proc updatecommits {} {
>> filerun $fd [list getcommitlines $fd $i $view 1]
>> incr viewactive($view)
>> set viewcomplete($view) 0
>> - set pending_select $mainheadid
>> + reset_pending_select {}
>
> This doesn't actually change anything, right? If so then I'd prefer
> the simple, direct assignment to calling a procedure that does the
> assignment.
I have a patch WIP that allows specifying a commit on the command line
to select instead of the head (I need it to enhance the git gui blame
UI). It makes the function somewhat more intelligent. I'll submit it
as soon as this series is sorted out.
>> @@ -4036,6 +4042,7 @@ proc layoutmore {} {
>> }
>> if {[info exists pending_select] &&
>> [commitinview $pending_select $curview]} {
>> + update
>> selectline [rowofcommit $pending_select] 1
>
> What does that update do? Would update idletasks be better?
That update forces Tk to recompute the widget dimensions. Otherwise
selectline sometimes gets all zeroes from yview, which makes it
compute really weird scrolling settings. Git-gui always does an update
before scrolling computations.
Alexander
^ permalink raw reply
* Re: git sequencer prototype
From: Stephan Beyer @ 2008-07-31 12:40 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Christian Couder, Daniel Barkalow
In-Reply-To: <alpine.LSU.1.00.0807301607070.3486@wbgn129.biozentrum.uni-wuerzburg.de>
Johannes Schindelin wrote:
> On Wed, 30 Jul 2008, Stephan Beyer wrote:
> > Johannes Schindelin wrote:
> > >
> > > > In the last patchset I mentioned the issue, that the prototype is
> > > > slow as hell. I know some bottlenecks, but I have not even tried to
> > > > change that, because this is no issue for the builtin.
> > >
> > > Why is it no issue for the builtin? Is it so much different from the
> > > prototype?
> >
> > As Sverre pointed out and as my "experiments" tried to show: the builtin
> > is just fast ;-)
>
> So why is it just so fast? Does it do things completely different than
> the shell prototype you presented?
Ah, that's the point of your question ;)
No, it's different in some minor parts, of course, but the important thing
is: it is C, not sh. That's what makes it faster.
Regards.
--
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F
^ permalink raw reply
* Re: Merging submodules
From: H.Merijn Brand @ 2008-07-31 12:39 UTC (permalink / raw)
To: Brian Gernhardt; +Cc: Git List, Lars Noschinski
In-Reply-To: <20080731092104.1a6ce8bd@pc09.procura.nl>
On Thu, 31 Jul 2008 09:21:04 +0200, "H.Merijn Brand"
<h.m.brand@xs4all.nl> wrote:
> I will now be playing with the results a bit. I have attached the
> script, in case you might want to use it in documentation or examples.
> For now, all the mods are hardcoded. No arguments and so on.
>
> Again, Thanks!
There is a slight problem with this merging approach. The path names
are as they are/were in the submodules. In module_a, foo.pl was without
a leading module_a/ path, and now after integration, it still is. Is it
possible to rethink this whole process that integrates/merges the
several git repo's in subfolders into the current folder, as-if they
would have been in this folder in the first place?
Short summary: History made me have the current situation
4gl
4gl/fnc
4gl/fnc/.git
4gl/foo
4gl/foo/.git
:
:
4gl/zlork
4gl/zlork/.git
Where I want
4gl
4gl/.git
4gl/fnc
4gl/foo
:
4gl/zlork
--
H.Merijn Brand Amsterdam Perl Mongers http://amsterdam.pm.org/
using & porting perl 5.6.2, 5.8.x, 5.10.x, 5.11.x on HP-UX 10.20, 11.00,
11.11, 11.23, and 11.31, SuSE 10.1, 10.2, and 10.3, AIX 5.2, and Cygwin.
http://mirrors.develooper.com/hpux/ http://www.test-smoke.org/
http://qa.perl.org http://www.goldmark.org/jeff/stupid-disclaimers/
^ permalink raw reply
* Re: linking libgit.a in C++ projects
From: Dmitry Potapov @ 2008-07-31 12:34 UTC (permalink / raw)
To: Petr Baudis; +Cc: cte, git
In-Reply-To: <20080731111446.GO32184@machine.or.cz>
On Thu, Jul 31, 2008 at 01:14:46PM +0200, Petr Baudis wrote:
>
> I don't think this is that big a problem; there are applications that
> are doing this already, e.g. cgit, and if you tie your application to
> a particular git version by for example making git a submodule of your
> source, this is pretty safe; it will just mean that you will have to
> do some non-trivial porting of your code to the new interface each time
> you update - but I think large changes in the interface are pretty rare
> in practice by now, and there shouldn't be much on the horizon either(?).
What you see as large changes depend on how well you know git internals.
Git develops very quickly and if someone who is trying to use libgit.a
does not follow git development closely, it may happen pretty soon that
even not so big changes will become a huge problem to accomadate them.
As result, the program may stick with an old Git version, and that puts
users of this program in the situation where they cannot use their
favorite frontend with new repositories.
> What would be the reason to disallow C++ users? The costs aren't that
> high, and (modulo, say, extern "C" { }) there should be no C-C++
> compatibility issues, right?
I mean that putting extern "C" { } around should be sufficient to use
this library in C++. But I see now some current headers contains some
C++ keywords and that causes the problem. So, yes, those headers should
be corrected if they become part of external available API. I am not
sure whether it makes sense to correct them now, but there are only
three places where C++ keywords are used:
diff.h:135:extern int diff_tree_sha1(const unsigned char *old, const
diff.h:137:extern int diff_root_tree_sha1(const unsigned char *new,
object.h:38:extern const char *typename(unsigned int type);
So, the patch should not be large, and it is up to Junio to decide
what to do about it.
Dmitry
^ permalink raw reply
* Re: [PATCH (GITK) 5/6] gitk: Fixed automatic row selection during load.
From: Paul Mackerras @ 2008-07-31 11:25 UTC (permalink / raw)
To: Alexander Gavrilov; +Cc: git
In-Reply-To: <200807271021.46661.angavrilov@gmail.com>
Alexander Gavrilov writes:
> - Switching views now actually preserves the selected commit.
> - Reloading (also Edit View) preserves the currently selected commit.
> - Initial selection does not produce weird scrolling.
>
> Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
I need a more detailed explanation of the rationale for the specific
changes you have made in the changelog.
As for the patch, it mostly looks good, but I have a few comments
below.
> +proc getcommits {selid} {
> global canv curview need_redisplay viewactive
>
> initlayout
> if {[start_rev_list $curview]} {
> + reset_pending_select $selid
Is there any significance to having the call to reset_pending_select
after the start_rev_list call (other than not setting pending_select
if start_rev_list fails)? I couldn't see any. If there is then it
should be noted in a comment and/or the patch description.
> @@ -503,7 +511,7 @@ proc updatecommits {} {
> filerun $fd [list getcommitlines $fd $i $view 1]
> incr viewactive($view)
> set viewcomplete($view) 0
> - set pending_select $mainheadid
> + reset_pending_select {}
This doesn't actually change anything, right? If so then I'd prefer
the simple, direct assignment to calling a procedure that does the
assignment.
> @@ -4036,6 +4042,7 @@ proc layoutmore {} {
> }
> if {[info exists pending_select] &&
> [commitinview $pending_select $curview]} {
> + update
> selectline [rowofcommit $pending_select] 1
What does that update do? Would update idletasks be better?
Thanks,
Paul.
^ permalink raw reply
* Re: markdown 2 man, was Re: Git Community Book
From: Abdelrazak Younes @ 2008-07-31 11:24 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Julian Phillips, Scott Chacon, Petr Baudis,
git list
In-Reply-To: <7vy73j418t.fsf@gitster.siamese.dyndns.org>
Hello,
Sorry for this irruption on this list. I am just a git user and casual
reader of this list. I thought I could share my thoughts about this as I
know a bit about document creation. Please ignore if this is not
appropriate.
Disclaimer: I am involved in LyX development, so anything I said will be
biased :-)
Junio C Hamano wrote:
> As I am not in "graphics and screencast" camp, I may probably not be able
> to offer much help improving his book, and I suspect some people on this
> list might feel the same way. But that's is Ok --- we are not dumping the
> User Manual.
IMHO, documentation is best written by users, not developer. So, again
IMHO, anything that could accommodate the _user_ for document writing
should be done. An enthusiastic user is more likely to spend time
writing documentation than a developer. For example, within the LyX
project, most writers and translator are not developer.
Asciidoc or Markdown are tools that accommodate the _developer_, not the
user. I understand that these markup language are ideally suited for in
source documentation (thought I personally much prefer Doxygen). I also
understand that launching a different application just to modify a line
or two in the user manual seems cumbersome for the developer but IMHO,
if you're serious about working on the documentation, you are not going
to change a line or two and launching an external application is no big
deal.
Now, about my shameless plug: LyX is ideally suited for structured
documentation writing :-)
Abdel.
^ permalink raw reply
* Re: linking libgit.a in C++ projects
From: cte @ 2008-07-31 11:20 UTC (permalink / raw)
To: Pedro Melo; +Cc: Dmitry Potapov, git
In-Reply-To: <0EC13CCC-0F7B-41A0-BEB2-67E6EC8D5E37@simplicidade.org>
>> Fortunately, g++ can compile C programs and link static libraries that
>> were compiled by C compilers, unless of course, they use C++ keywords.
>> I don't think it is unreasonable to rename the _very few_ C++ keywords
>> in git's source in the interest of allowing C++ projects to leverage
>> libgit.
>
> I think the point Dmitry was trying to make is that you should compile
> libgit as C, using gcc, and then link it with your C++/Objective C code.
>
> No patch is required to git, only to your makefile/xcode project file.
The git .h files must be in your include path, and must not contain
C++ keywords in order to link against libgit.a.
^ permalink raw reply
* Re: linking libgit.a in C++ projects
From: Petr Baudis @ 2008-07-31 11:20 UTC (permalink / raw)
To: Pedro Melo; +Cc: cte, Dmitry Potapov, git
In-Reply-To: <0EC13CCC-0F7B-41A0-BEB2-67E6EC8D5E37@simplicidade.org>
On Thu, Jul 31, 2008 at 12:16:45PM +0100, Pedro Melo wrote:
> I think the point Dmitry was trying to make is that you should compile
> libgit as C, using gcc, and then link it with your C++/Objective C code.
>
> No patch is required to git, only to your makefile/xcode project file.
libgit has a certain (albeit currently unofficial and non-settled) API
and the API is defined in header files that must be eatable by C++
compilers in order to do this.
Petr "Pasky" Baudis
^ permalink raw reply
* Re: linking libgit.a in C++ projects
From: cte @ 2008-07-31 11:18 UTC (permalink / raw)
To: Petr Baudis; +Cc: Dmitry Potapov, git
In-Reply-To: <20080731111446.GO32184@machine.or.cz>
> What would be the reason to disallow C++ users? The costs aren't that
> high, and (modulo, say, extern "C" { }) there should be no C-C++
> compatibility issues, right?
Exactly. It works just great for me in XCode 3.1 on OS X.
^ permalink raw reply
* Re: linking libgit.a in C++ projects
From: Pedro Melo @ 2008-07-31 11:16 UTC (permalink / raw)
To: cte; +Cc: Dmitry Potapov, git
In-Reply-To: <ac9f0f090807310410u461f5584ved74769d8452c539@mail.gmail.com>
On Jul 31, 2008, at 12:10 PM, cte wrote:
> On Thu, Jul 31, 2008 at 3:57 AM, Dmitry Potapov <dpotapov@gmail.com>
> wrote:
>> On Thu, Jul 31, 2008 at 02:53:37AM -0700, cte wrote:
>>> However, the git source uses a few reserved C++ keywords; namely
>>> 'typename', and 'new'.
>>
>> Because this source code are meant to be compiled by C and not by C+
>> +!
>> Even if we will have real git library for other applications to use,
>> it still be compiled only by C. Thus, C++ keywords are not issue.
>
[...]
> Fortunately, g++ can compile C programs and link static libraries that
> were compiled by C compilers, unless of course, they use C++ keywords.
> I don't think it is unreasonable to rename the _very few_ C++ keywords
> in git's source in the interest of allowing C++ projects to leverage
> libgit.
I think the point Dmitry was trying to make is that you should compile
libgit as C, using gcc, and then link it with your C++/Objective C code.
No patch is required to git, only to your makefile/xcode project file.
Best regards,
--
Pedro Melo
Blog: http://www.simplicidade.org/notes/
XMPP ID: melo@simplicidade.org
Use XMPP!
^ permalink raw reply
* Re: linking libgit.a in C++ projects
From: Petr Baudis @ 2008-07-31 11:14 UTC (permalink / raw)
To: Dmitry Potapov; +Cc: cte, git
In-Reply-To: <20080731105727.GF7008@dpotapov.dyndns.org>
On Thu, Jul 31, 2008 at 02:57:27PM +0400, Dmitry Potapov wrote:
> On Thu, Jul 31, 2008 at 02:53:37AM -0700, cte wrote:
> > I'm writing a git gui for OS X using cocoa/Objective-C++, and rather
> > than being lame and parsing the output the various git commands, I'm
> > using libgit.a to provide all of the needed functionality for my app.
>
> Don't do that! libgit.a is an internal library used solely to build
> git binaries. It means that its interface can be cahnged at any time.
I don't think this is that big a problem; there are applications that
are doing this already, e.g. cgit, and if you tie your application to
a particular git version by for example making git a submodule of your
source, this is pretty safe; it will just mean that you will have to
do some non-trivial porting of your code to the new interface each time
you update - but I think large changes in the interface are pretty rare
in practice by now, and there shouldn't be much on the horizon either(?).
> > However, the git source uses a few reserved C++ keywords; namely
> > 'typename', and 'new'.
>
> Because this source code are meant to be compiled by C and not by C++!
> Even if we will have real git library for other applications to use,
> it still be compiled only by C. Thus, C++ keywords are not issue.
What would be the reason to disallow C++ users? The costs aren't that
high, and (modulo, say, extern "C" { }) there should be no C-C++
compatibility issues, right?
--
Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name. -- Ken Thompson and Dennis M. Ritchie
^ permalink raw reply
* Re: linking libgit.a in C++ projects
From: cte @ 2008-07-31 11:10 UTC (permalink / raw)
To: Dmitry Potapov; +Cc: git
In-Reply-To: <20080731105727.GF7008@dpotapov.dyndns.org>
On Thu, Jul 31, 2008 at 3:57 AM, Dmitry Potapov <dpotapov@gmail.com> wrote:
> On Thu, Jul 31, 2008 at 02:53:37AM -0700, cte wrote:
>> I'm writing a git gui for OS X using cocoa/Objective-C++, and rather
>> than being lame and parsing the output the various git commands, I'm
>> using libgit.a to provide all of the needed functionality for my app.
>
> Don't do that! libgit.a is an internal library used solely to build
> git binaries. It means that its interface can be cahnged at any time.
> Though, there is an idea of creating the real git library that other
> applications can use, but AFAIK no one is working on it. So parsing
> output is the only correct solution right now. In fact, it is not
> difficult to do, because most plumbing commands are rather flexibly
> in what they output and how.
I'm not worried about the interfaces changing; the gui is tied to a
particular version of git, and I will update the code that calls into
libgit I pull new changes from the mainline into my local clone. Also,
who's to say that the output of the various commands won't change
formats with future releases of git? There is no correct solution if
you are worried about forward compatibility, unless a well defined API
is created (which would be sweet btw, but is probably not a priority).
>> However, the git source uses a few reserved C++ keywords; namely
>> 'typename', and 'new'.
>
> Because this source code are meant to be compiled by C and not by C++!
> Even if we will have real git library for other applications to use,
> it still be compiled only by C. Thus, C++ keywords are not issue.
Clearly ;)
Fortunately, g++ can compile C programs and link static libraries that
were compiled by C compilers, unless of course, they use C++ keywords.
I don't think it is unreasonable to rename the _very few_ C++ keywords
in git's source in the interest of allowing C++ projects to leverage
libgit.
^ permalink raw reply
* Re: [PATCH v3] Advertise the ability to abort a commit
From: Jeff King @ 2008-07-31 11:09 UTC (permalink / raw)
To: Petr Baudis; +Cc: Junio C Hamano, Anders Melchiorsen, git
In-Reply-To: <20080731105539.GM32184@machine.or.cz>
On Thu, Jul 31, 2008 at 12:55:39PM +0200, Petr Baudis wrote:
> > if (cleanup_mode == CLEANUP_ALL)
> > - fprintf(fp, "not be included)\n");
> > + fprintf(fp,
> > + " Lines starting\n"
> > + "# with '#' will be ignored, and an empty"
> > + " message aborts the commit.\n");
> > else /* CLEANUP_SPACE, that is. */
> > - fprintf(fp, "be kept.\n"
> > - "# You can remove them yourself if you want to)\n");
> > + fprintf(fp,
> > + " Lines starting\n"
> > + "# with '#' will be kept; you may remove them"
> > + " yourself if you want to.\n"
> > + "# An empty message aborts the commit.\n");
> > if (only_include_assumed)
> > fprintf(fp, "# %s\n", only_include_assumed);
> >
>
> This is rather funny-looking; you print _one_ fragment of the common
> string by a common fprintf, but then repeat _second_ fragment of the
> still-common string in a per-case fprintf. Can't we at least split this
> on the line boundary, if not do something loosely like this?
I just broke it by sentence, thinking that followed the semantics more
clearly (i.e., the first fprintf says one thing, then the second says
another; however, we must say the second one differently depending on
the case). I almost just split the whole paragraph by cleanup case,
allowing each to be worded and wrapped as most appropriate.
> fprintf(fp,
> "\n"
> "# Please enter the commit message for your "
> "changes. Lines starting\n"
> "# with a '#' will be %s "
> "and an empty message aborts the commit\n",
> cleanup_mode == CLEANUP_ALL ? "ignored,"
> /* CLEANUP_SPACE */ : "kept (you may remove them "
> "yourself if you want to)\n#");
I did something like that before submitting, but decided against it
because:
- I found mine more readable, since it is hard to see in yours exactly
where there will be a linebreak.
- I actually changed the phrasing for the second one. Since we
introduce another clause into the sentence in the CLEANUP_SPACE
case, it makes sense to start another sentence for the final point.
-Peff
^ permalink raw reply
* Re: [PATCH] 64bit issue in test-parse-options.c
From: Petr Baudis @ 2008-07-31 11:07 UTC (permalink / raw)
To: Pierre Habouzit, H.Merijn Brand, git
In-Reply-To: <20080730140523.GC31392@artemis.madism.org>
On Wed, Jul 30, 2008 at 04:05:23PM +0200, Pierre Habouzit wrote:
> On Wed, Jul 30, 2008 at 12:44:52PM +0000, H.Merijn Brand wrote:
> > On Wed, 30 Jul 2008 14:37:13 +0200, Pierre Habouzit
> > <madcoder@debian.org> wrote:
> > > long is wrong in the first place, parse-opt only uses ints.
> >
> > If I change it to int, the date parsing goes bogus.
>
> That is because OPT_DATE indeed expect a long (why not a time_t I don't
> know btw... but time_t is a long on linux so it doesn't change things a
> lot).
Still, I think converting to time_t for timestamps would be a good
cleanup; I have added it to http://git.or.cz/gitwiki/Janitor.
Petr "Pasky" Baudis
^ permalink raw reply
* Re: linking libgit.a in C++ projects
From: Dmitry Potapov @ 2008-07-31 10:57 UTC (permalink / raw)
To: cte; +Cc: git
In-Reply-To: <ac9f0f090807310253v1d97e2a1n4ddf34aa4fdc79f0@mail.gmail.com>
On Thu, Jul 31, 2008 at 02:53:37AM -0700, cte wrote:
> I'm writing a git gui for OS X using cocoa/Objective-C++, and rather
> than being lame and parsing the output the various git commands, I'm
> using libgit.a to provide all of the needed functionality for my app.
Don't do that! libgit.a is an internal library used solely to build
git binaries. It means that its interface can be cahnged at any time.
Though, there is an idea of creating the real git library that other
applications can use, but AFAIK no one is working on it. So parsing
output is the only correct solution right now. In fact, it is not
difficult to do, because most plumbing commands are rather flexibly
in what they output and how.
> However, the git source uses a few reserved C++ keywords; namely
> 'typename', and 'new'.
Because this source code are meant to be compiled by C and not by C++!
Even if we will have real git library for other applications to use,
it still be compiled only by C. Thus, C++ keywords are not issue.
Dmitry
^ permalink raw reply
* Re: [PATCH v3] Advertise the ability to abort a commit
From: Petr Baudis @ 2008-07-31 10:55 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Anders Melchiorsen, git
In-Reply-To: <20080731073609.GA8049@sigill.intra.peff.net>
On Thu, Jul 31, 2008 at 03:36:09AM -0400, Jeff King wrote:
> builtin-commit.c | 19 ++++++++++++-------
> t/t7502-commit.sh | 11 +++++------
> 2 files changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/builtin-commit.c b/builtin-commit.c
> index f7c053a..b783e6e 100644
> --- a/builtin-commit.c
> +++ b/builtin-commit.c
> @@ -554,14 +554,18 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
>
> fprintf(fp,
> "\n"
> - "# Please enter the commit message for your changes.\n"
> - "# To abort the commit, use an empty commit message.\n"
> - "# (Comment lines starting with '#' will ");
> + "# Please enter the commit message for your changes.");
> if (cleanup_mode == CLEANUP_ALL)
> - fprintf(fp, "not be included)\n");
> + fprintf(fp,
> + " Lines starting\n"
> + "# with '#' will be ignored, and an empty"
> + " message aborts the commit.\n");
> else /* CLEANUP_SPACE, that is. */
> - fprintf(fp, "be kept.\n"
> - "# You can remove them yourself if you want to)\n");
> + fprintf(fp,
> + " Lines starting\n"
> + "# with '#' will be kept; you may remove them"
> + " yourself if you want to.\n"
> + "# An empty message aborts the commit.\n");
> if (only_include_assumed)
> fprintf(fp, "# %s\n", only_include_assumed);
>
This is rather funny-looking; you print _one_ fragment of the common
string by a common fprintf, but then repeat _second_ fragment of the
still-common string in a per-case fprintf. Can't we at least split this
on the line boundary, if not do something loosely like this?
fprintf(fp,
"\n"
"# Please enter the commit message for your "
"changes. Lines starting\n"
"# with a '#' will be %s "
"and an empty message aborts the commit\n",
cleanup_mode == CLEANUP_ALL ? "ignored,"
/* CLEANUP_SPACE */ : "kept (you may remove them "
"yourself if you want to)\n#");
--
Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name. -- Ken Thompson and Dennis M. Ritchie
^ permalink raw reply
* Re: [PATCH] git-svn now work with crlf convertion enabled.
From: Dmitry Potapov @ 2008-07-31 10:45 UTC (permalink / raw)
To: Alexander Litvinov; +Cc: git, Eric Wong
In-Reply-To: <200807311257.49108.litvinov2004@gmail.com>
On Thu, Jul 31, 2008 at 12:57:48PM +0700, Alexander Litvinov wrote:
>
> git-svn fetch files with this patch but I have found that git-svn use
> git-hash-object and provide file name to store into stdin. As far as file is
> a temp file git-hash-object can't correctly apply crlf convertion for the
> file.
It does not look to be true. I did the following test:
mkdir hash_test
cd hash_test
git init
cat <<\=== > hash_test.pl
#!/usr/bin/env perl
use File::Temp qw/tempfile/;
my ($tmp_fh, $tmp_filename) = File::Temp::tempfile(UNLINK => 1);
print $tmp_fh "Hi\r\n";
$tmp_fh->flush;
system ("echo $tmp_filename | git hash-object --stdin-paths");
===
git config core.autocrlf true
perl hash_test.pl
git config core.autocrlf false
perl hash_test.pl
and the output was
b14df6442ea5a1b382985a6549b85d435376c351
ea6b6afbc2cbed0eb8c0f7561286ab72f349416c
which means that the autocrlf conversion is done for temporary
files created by perl. (I tested it on Linux and Windows/Cygwin).
In any case, I believe the right solution should be adding a
new option to git-hash-object to disable any conversion.
Dmitry
^ permalink raw reply
* Re: git blame not respecting --find-copies-harder ?
From: Jeff King @ 2008-07-31 10:33 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: Junio C Hamano, Git Mailinglist
In-Reply-To: <20080731102223.GD12867@artemis.madism.org>
On Thu, Jul 31, 2008 at 12:22:23PM +0200, Pierre Habouzit wrote:
> > [Pierre, you have the bogus MFT header that people are often complaining
> > about. Either I have to do extra work to fix the headers, or more people
> > end up in the To: field. I don't personally care about the latter, but
> > it seems that others organize their mail based on To versus Cc]
>
> [ err if you do a list reply, nobody ends up in To:... but oh well I
> can probably remove it, it'll just make my own mutt git folder
> awkwardly colorized. ]
I can't do a list reply, as mutt sees no list (and I didn't bother
setting up a 'subscribe' variable, since I didn't want to generate MFTs
in the first place). However, I just realized that mutt has an
honor_followup_to, which will do what I want (AFAICT, there is no point
in honoring MFT when using the conventions of this mailing list).
But I wonder if it is as simple in other clients.
-Peff
^ permalink raw reply
* Re: git blame not respecting --find-copies-harder ?
From: Pierre Habouzit @ 2008-07-31 10:22 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, sverre, Björn Steinbrink,
Stephen R. van den Berg, Git Mailinglist
In-Reply-To: <20080731093410.GA21396@sigill.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 3074 bytes --]
> [Pierre, you have the bogus MFT header that people are often complaining
> about. Either I have to do extra work to fix the headers, or more people
> end up in the To: field. I don't personally care about the latter, but
> it seems that others organize their mail based on To versus Cc]
[ err if you do a list reply, nobody ends up in To:... but oh well I
can probably remove it, it'll just make my own mutt git folder
awkwardly colorized. ]
On Thu, Jul 31, 2008 at 09:34:10AM +0000, Jeff King wrote:
> On Thu, Jul 31, 2008 at 11:15:15AM +0200, Pierre Habouzit wrote:
> > --no-walk alters how add_pending_object_with_mode works, which is called
> > by handle_revision_arg. IOW, <ref1> --no-walk <ref2> is not the same as
> > --no-walk <ref1> <ref2>. IOW reference arguments and --no-walk ordering
> > must be preserved, IOW --no-walk is a pseudo-argument like --all --tags
> > or --remote are.
>
> Ah, OK. Then that makes sense, then. And I definitely feel that the
> patch I posted is the right thing to do,
Yeah I was convinced the first time I saw it already :)
-- >8 --
allow "non-option" revision options in parse_option-enabled commands
Commands which use parse_options but also call
setup_revisions must do their parsing in a two step process:
1. first, they parse all options. Anything unknown goes to
parse_revision_opt (which calls handle_revision_opt),
which may claim the option or say "I don't recognize
this"
2. the non-option remainder goes to setup_revisions to
actually get turned into revisions
Some revision options are "non-options" in that they must be
parsed in order with their revision counterparts in
setup_revisions. For example, "--all" functions as a
pseudo-option expanding to all refs, and "--no-walk" affects
refs after it on the command line, but not before. The
revision option parser in step 1 recognizes such options and
sets them aside for later parsing by setup_revisions.
However, the return value used from handle_revision_opt
indicated "I didn't recognize this", which was wrong. It
did, and it took appropriate action (even though that action
was just deferring it for later parsing). Thus it should
return "yes, I recognized this."
Previously, these pseudo-options generated an error when
used with parse_options parsers (currently just blame and
shortlog). With this patch, they should work fine, enabling
things like "git shortlog --all".
Signed-off-by: Jeff King <peff@peff.net>
Acked-By: Pierre Habouzit <madcoder@debian.org>
---
revision.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/revision.c b/revision.c
index a843c42..eaa5572 100644
--- a/revision.c
+++ b/revision.c
@@ -1002,7 +1002,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
!strcmp(arg, "--no-walk") || !strcmp(arg, "--do-walk"))
{
unkv[(*unkc)++] = arg;
- return 0;
+ return 1;
}
if (!prefixcmp(arg, "--max-count=")) {
--
1.6.0.rc1.197.gc57ac6
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply related
* linking libgit.a in C++ projects
From: cte @ 2008-07-31 9:53 UTC (permalink / raw)
To: git
I'm writing a git gui for OS X using cocoa/Objective-C++, and rather
than being lame and parsing the output the various git commands, I'm
using libgit.a to provide all of the needed functionality for my app.
However, the git source uses a few reserved C++ keywords; namely
'typename', and 'new'. So, I was wondering if it is worth submitting a
patch to fix these issues... I'm asking because I'm new to the whole
open source thing, and I don't want to get yelled at by the git
maintainers for submitting stupid patches that no one in their right
mind would accept :)
Thanks!
^ permalink raw reply
* Re: git blame not respecting --find-copies-harder ?
From: Jeff King @ 2008-07-31 9:34 UTC (permalink / raw)
To: Pierre Habouzit
Cc: Junio C Hamano, sverre, Björn Steinbrink,
Stephen R. van den Berg, Git Mailinglist
In-Reply-To: <20080731091515.GC12867@artemis.madism.org>
[Pierre, you have the bogus MFT header that people are often complaining
about. Either I have to do extra work to fix the headers, or more people
end up in the To: field. I don't personally care about the latter, but
it seems that others organize their mail based on To versus Cc]
On Thu, Jul 31, 2008 at 11:15:15AM +0200, Pierre Habouzit wrote:
> --no-walk alters how add_pending_object_with_mode works, which is called
> by handle_revision_arg. IOW, <ref1> --no-walk <ref2> is not the same as
> --no-walk <ref1> <ref2>. IOW reference arguments and --no-walk ordering
> must be preserved, IOW --no-walk is a pseudo-argument like --all --tags
> or --remote are.
Ah, OK. Then that makes sense, then. And I definitely feel that the
patch I posted is the right thing to do, then. So here is a patch with
commit message. The commit message to patch ratio is ridiculous, but
obviously this fix took some thinking, so it seems best to document it.
-- >8 --
allow "non-option" revision options in parse_option-enabled commands
Commands which use parse_options but also call
setup_revisions must do their parsing in a two step process:
1. first, they parse all options. Anything unknown goes to
parse_revision_opt (which calls handle_revision_opt),
which may claim the option or say "I don't recognize
this"
2. the non-option remainder goes to setup_revisions to
actually get turned into revisions
Some revision options are "non-options" in that they must be
parsed in order with their revision counterparts in
setup_revisions. For example, "--all" functions as a
pseudo-option expanding to all refs, and "--no-walk" affects
refs after it on the command line, but not before. The
revision option parser in step 1 recognizes such options and
sets them aside for later parsing by setup_revisions.
However, the return value used from handle_revision_opt
indicated "I didn't recognize this", which was wrong. It
did, and it took appropriate action (even though that action
was just deferring it for later parsing). Thus it should
return "yes, I recognized this."
Previously, these pseudo-options generated an error when
used with parse_options parsers (currently just blame and
shortlog). With this patch, they should work fine, enabling
things like "git shortlog --all".
Signed-off-by: Jeff King <peff@peff.net>
---
revision.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/revision.c b/revision.c
index a843c42..eaa5572 100644
--- a/revision.c
+++ b/revision.c
@@ -1002,7 +1002,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
!strcmp(arg, "--no-walk") || !strcmp(arg, "--do-walk"))
{
unkv[(*unkc)++] = arg;
- return 0;
+ return 1;
}
if (!prefixcmp(arg, "--max-count=")) {
--
1.6.0.rc1.197.gc57ac6
^ permalink raw reply related
* Re: git blame not respecting --find-copies-harder ?
From: Pierre Habouzit @ 2008-07-31 9:15 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, sverre, Björn Steinbrink,
Stephen R. van den Berg, Git Mailinglist
In-Reply-To: <20080731090348.GB20691@sigill.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 1976 bytes --]
On Thu, Jul 31, 2008 at 09:03:49AM +0000, Jeff King wrote:
> Not necessarily. The logic there goes:
>
> 1. it's not a revision option, so pass to diff
> 2. it's not a diff opt, so it is unknown; we parsed no options
> 3. the caller sees we failed to parse, so it barfs
>
> but the logic here is:
>
> 1. it _is_ a revision option. Our handling of it is just a little odd,
> in that we need to parse it later, when we are in setup_revisions.
> So put it aside for now as a "revision" that just happens to look
> like an option.
> 2. caller sees we parsed, and doesn't complain
> 3. caller later passes the "revision" to setup_revisions, which knows
> what to do
>
> Now my patch doesn't just operate on "--all", but rather several other
> options including --no-walk. And maybe that is not right, and --all
> should be handled separately (though I think --remotes would follow the
> same logic). I'm not really sure why --no-walk is separated like that.
--no-walk alters how add_pending_object_with_mode works, which is called
by handle_revision_arg. IOW, <ref1> --no-walk <ref2> is not the same as
--no-walk <ref1> <ref2>. IOW reference arguments and --no-walk ordering
must be preserved, IOW --no-walk is a pseudo-argument like --all --tags
or --remote are.
It's okay for parse_revision_opt to take out any option that doesn't
alter handle_revision_arg (as for parse_option based parsers it's the
sole thing setup_revision will be doing: consuming revisions in a loop
with handle_revision_arg), but only those.
The full list (that I made when I wrote parse_revision_opt) is actually:
pseudo arguments: --all --branches --tags --remotes --reflog
combining operator: --not
behavior modifying: --no-walk --do-walk
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: git blame not respecting --find-copies-harder ?
From: Pierre Habouzit @ 2008-07-31 9:06 UTC (permalink / raw)
To: Junio C Hamano, Jeff King, sverre, Björn Steinbrink,
Stephen R. van den Berg
In-Reply-To: <20080731090146.GA12867@artemis.madism.org>
[-- Attachment #1: Type: text/plain, Size: 1995 bytes --]
On Thu, Jul 31, 2008 at 09:01:46AM +0000, Pierre Habouzit wrote:
> On Thu, Jul 31, 2008 at 08:35:33AM +0000, Junio C Hamano wrote:
> > Jeff King <peff@peff.net> writes:
> >
> > > On Thu, Jul 31, 2008 at 12:36:59AM -0700, Junio C Hamano wrote:
> > >
> > >> Alas, shortlog does not take --all. Yes, I know
> > >>
> > >> git log --since=3.day --all | git shortlog | sort | uniq -c
> > >>
> > >> is an obvious workaround, but it is mildly irritating.
> > >
> > > Hmm. Could it be as simple as:
> > >
> > > diff --git a/revision.c b/revision.c
> > > index a843c42..eaa5572 100644
> > > --- a/revision.c
> > > +++ b/revision.c
> > > @@ -1002,7 +1002,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
> > > !strcmp(arg, "--no-walk") || !strcmp(arg, "--do-walk"))
> > > {
> > > unkv[(*unkc)++] = arg;
> > > - return 0;
> > > + return 1;
> > > }
> > >
> > > if (!prefixcmp(arg, "--max-count=")) {
> > >
> > > That is, handle_revision_opt says "yes we parsed this, and it should be
> > > gone" even though it still gets stuck in the "unknown" section to be
> > > parsed by setup_revisions later.
>
> Ack this is a correct fix. I wonder how this even works with other
> commands that use --all and stuff like that.
Oh actually I know: the parse_revision_opt machinery (that is buggy
because of this 0 result) is used in git-blame where --all is
meaningless anyways, and ... git-shortlog only. The legacy way to parse
revision options does not treats `0' answers differently from `1'
actually, I wonder why, because this is probably wrong.
This was a regression, I suppose prior to its parse-optification
git-shortlog accepted --all (and I see no valid reason for it not to).
Thanks a lot to Jeff for the good catch.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: git blame not respecting --find-copies-harder ?
From: Jeff King @ 2008-07-31 9:05 UTC (permalink / raw)
To: Pierre Habouzit, Junio C Hamano, sverre, Björn Steinbrink,
Stephen R. van den Berg
In-Reply-To: <20080731090146.GA12867@artemis.madism.org>
On Thu, Jul 31, 2008 at 11:01:46AM +0200, Pierre Habouzit wrote:
> Ack this is a correct fix. I wonder how this even works with other
> commands that use --all and stuff like that.
--all is parsed separately in setup_revisions _before_ we call
handle_revision_opt. So the only codepath hitting that is parse_options
users who call parse_revision_opt (i.e., blame and shortlog).
-Peff
^ permalink raw reply
* Re: git blame not respecting --find-copies-harder ?
From: Jeff King @ 2008-07-31 9:03 UTC (permalink / raw)
To: Junio C Hamano
Cc: sverre, Björn Steinbrink, Stephen R. van den Berg,
Git Mailinglist
In-Reply-To: <7v8wvizc16.fsf@gitster.siamese.dyndns.org>
On Thu, Jul 31, 2008 at 01:35:33AM -0700, Junio C Hamano wrote:
> > @@ -1002,7 +1002,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
> > !strcmp(arg, "--no-walk") || !strcmp(arg, "--do-walk"))
> > {
> > unkv[(*unkc)++] = arg;
> > - return 0;
> > + return 1;
> > }
> >
> > if (!prefixcmp(arg, "--max-count=")) {
> >
> > That is, handle_revision_opt says "yes we parsed this, and it should be
> > gone" even though it still gets stuck in the "unknown" section to be
> > parsed by setup_revisions later.
>
> Hmm, wouldn't that suggest it needs to return 1 when an option candidate
> given to diff_opt_parse() turns out not to be a diff option and stuffed
> back to unkv[] at the end of this function?
Not necessarily. The logic there goes:
1. it's not a revision option, so pass to diff
2. it's not a diff opt, so it is unknown; we parsed no options
3. the caller sees we failed to parse, so it barfs
but the logic here is:
1. it _is_ a revision option. Our handling of it is just a little odd,
in that we need to parse it later, when we are in setup_revisions.
So put it aside for now as a "revision" that just happens to look
like an option.
2. caller sees we parsed, and doesn't complain
3. caller later passes the "revision" to setup_revisions, which knows
what to do
Now my patch doesn't just operate on "--all", but rather several other
options including --no-walk. And maybe that is not right, and --all
should be handled separately (though I think --remotes would follow the
same logic). I'm not really sure why --no-walk is separated like that.
-Peff
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox