Re: [PATCH gitk 0/4] gitk support for git log -L
Junio C Hamano gits...@pobox.com writes:
Thomas Rast tr...@inf.ethz.ch writes:
Whether the option value is a separate argument in argv, or directly
stuck to the option.
stuck: gitk -L:foo:main.c
unstuck: gitk -L :foo:main.c
Existing gitk chokes on 'gitk -S foo', but works with 'git -Sfoo'.
I somehow thought that we encourage the stuck/sticked form, to
reduce things the users need to remember to cope better with options
with optional value.
I just looked into this again, to get it rolling.
Am I reading you correctly as saying that any support for the unstuck
form is entirely coincidental, and it's okay to support only the stuck
version in new gitk?
Note that the support for 'git log -L' supports both, and it irks me
that the user can't switch back and forth between using 'gitk' and 'git
log' while leaving the rest of the command intact.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH gitk 0/4] gitk support for git log -L
Thomas Rast wrote: Junio C Hamano gits...@pobox.com writes: Thomas Rast tr...@inf.ethz.ch writes: Existing gitk chokes on 'gitk -S foo', but works with 'git -Sfoo'. I somehow thought that we encourage the stuck/sticked form, to reduce things the users need to remember to cope better with options with optional value. I just looked into this again, to get it rolling. Am I reading you correctly as saying that any support for the unstuck form is entirely coincidental, and it's okay to support only the stuck version in new gitk? Sort of. :) gitcli(7) says that the sticked form is to be preferred when you are scripting git. But most git commands use parse-options, which of course supports both forms and makes life easier for humans. Support for just the sticked form is better than nothing, especially if the gitk(1) manpage gains a note about it. In the long run I guess the ideal would be to add a parse-options-like library to the tcl support. My two cents, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH gitk 0/4] gitk support for git log -L
Paul Mackerras pau...@samba.org writes:
Hi Thomas,
On Wed, Jul 31, 2013 at 03:17:41PM +0200, Thomas Rast wrote:
Jens Lehmann jens.lehm...@web.de writes:
Am 29.07.2013 21:37, schrieb Thomas Rast:
Thomas Rast tr...@inf.ethz.ch writes:
Thomas Rast tr...@inf.ethz.ch writes:
Now that git log -L has hit master, I figure it's time to discuss the
corresponding change to gitk.
[...]
One thing I worry about is having gitk storing in memory not just the
history graph but also all the diffs (assuming I have understood
correctly what you're doing). Gitk's memory consumption is already
pretty large. However, I can't see an alternative at this point.
I don't think there is one. log -L is pretty much an all or nothing
thing at this point. I suppose if we really found that the diffs are
regularly too big to be manageable for gitk, we could invent a porcelain
mode where 'log -L' just prints the detected commits and corresponding
line ranges, and then have a new option to diff-tree to let it again
filter that range.
But note that ordinary 'git log -L' also buffers the entire set of diffs
within less. The memory consumption of gitk to hold the same diffs in
memory should be only a small factor of what less uses in the same
scenario. Furthermore, users will typically ask for a small region of
code (one function, or some such), so the diffs themselves are usually
quite small, nowhere near the size of the full commit diffs.
Unfortunately it's turning out to be harder than I hoped. gitk runs the
arguments through git-rev-parse, which only knows that -n gets an
unstuck argument. Consequently, gitk accepts an unstuck -n but only
stuck forms of -S and -G.
Excuse my ignorance, but what do you mean by stuck vs. unstuck?
Whether the option value is a separate argument in argv, or directly
stuck to the option.
stuck: gitk -L:foo:main.c
unstuck: gitk -L :foo:main.c
Existing gitk chokes on 'gitk -S foo', but works with 'git -Sfoo'.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH gitk 0/4] gitk support for git log -L
Thomas Rast tr...@inf.ethz.ch writes: Whether the option value is a separate argument in argv, or directly stuck to the option. stuck: gitk -L:foo:main.c unstuck: gitk -L :foo:main.c Existing gitk chokes on 'gitk -S foo', but works with 'git -Sfoo'. I somehow thought that we encourage the stuck/sticked form, to reduce things the users need to remember to cope better with options with optional value. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH gitk 0/4] gitk support for git log -L
Hi Thomas, On Wed, Jul 31, 2013 at 03:17:41PM +0200, Thomas Rast wrote: Jens Lehmann jens.lehm...@web.de writes: Am 29.07.2013 21:37, schrieb Thomas Rast: Thomas Rast tr...@inf.ethz.ch writes: Thomas Rast tr...@inf.ethz.ch writes: Now that git log -L has hit master, I figure it's time to discuss the corresponding change to gitk. Paul, any news on this? Any chance we can get it into the next release, since that will also be the first release to ship with 'git log -L'? Jens pointed out privately that the handling of unstuck -L options is unfortunate, to put it mildly. I'll send a reroll. But as soon as that is fixed I'd really like to see this applied, as I think gitk is the perfect tool to show history information. One thing I worry about is having gitk storing in memory not just the history graph but also all the diffs (assuming I have understood correctly what you're doing). Gitk's memory consumption is already pretty large. However, I can't see an alternative at this point. Unfortunately it's turning out to be harder than I hoped. gitk runs the arguments through git-rev-parse, which only knows that -n gets an unstuck argument. Consequently, gitk accepts an unstuck -n but only stuck forms of -S and -G. Excuse my ignorance, but what do you mean by stuck vs. unstuck? Thanks, Paul. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH gitk 0/4] gitk support for git log -L
Jens Lehmann jens.lehm...@web.de writes:
Am 29.07.2013 21:37, schrieb Thomas Rast:
Thomas Rast tr...@inf.ethz.ch writes:
Thomas Rast tr...@inf.ethz.ch writes:
Now that git log -L has hit master, I figure it's time to discuss the
corresponding change to gitk.
Paul, any news on this? Any chance we can get it into the next release,
since that will also be the first release to ship with 'git log -L'?
Jens pointed out privately that the handling of unstuck -L options is
unfortunate, to put it mildly. I'll send a reroll.
But as soon as that is fixed I'd really like to see this applied, as
I think gitk is the perfect tool to show history information.
Unfortunately it's turning out to be harder than I hoped. gitk runs the
arguments through git-rev-parse, which only knows that -n gets an
unstuck argument. Consequently, gitk accepts an unstuck -n but only
stuck forms of -S and -G.
Fixing it through git-rev-parse feels wrong; rev-parse is supposed to
know about rev-list options, but -S and -G only make sense in
diff-generating walks, and -L only makes any sense at all for git-log.
I'm tempted to leave it at the existing patches for now. That does mean
that -L can only be used in the stuck form; it's the same for -S and -G
already. Then in a later series we can change gitk's argument parsing
to properly treat the options directly, passing only the remaining
arguments through to rev-parse to use the usual revision/filename
distinction logic.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH gitk 0/4] gitk support for git log -L
Thomas Rast tr...@inf.ethz.ch writes:
Thomas Rast tr...@inf.ethz.ch writes:
Now that git log -L has hit master, I figure it's time to discuss the
corresponding change to gitk.
Paul, any news on this? Any chance we can get it into the next release,
since that will also be the first release to ship with 'git log -L'?
Jens pointed out privately that the handling of unstuck -L options is
unfortunate, to put it mildly. I'll send a reroll.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH gitk 0/4] gitk support for git log -L
Am 29.07.2013 21:37, schrieb Thomas Rast: Thomas Rast tr...@inf.ethz.ch writes: Thomas Rast tr...@inf.ethz.ch writes: Now that git log -L has hit master, I figure it's time to discuss the corresponding change to gitk. Paul, any news on this? Any chance we can get it into the next release, since that will also be the first release to ship with 'git log -L'? Jens pointed out privately that the handling of unstuck -L options is unfortunate, to put it mildly. I'll send a reroll. But as soon as that is fixed I'd really like to see this applied, as I think gitk is the perfect tool to show history information. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH gitk 0/4] gitk support for git log -L
Thomas Rast tr...@inf.ethz.ch writes:
Now that git log -L has hit master, I figure it's time to discuss the
corresponding change to gitk.
Paul, any news on this? Any chance we can get it into the next release,
since that will also be the first release to ship with 'git log -L'?
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
