* Re: [PATCH] rebase -i: proper prepare-commit-msg hook argument when squashing
From: Jeff King @ 2008-10-03 6:01 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Shawn O. Pearce, git
In-Reply-To: <1222992501-943-1-git-send-email-szeder@ira.uka.de>
On Fri, Oct 03, 2008 at 02:08:21AM +0200, SZEDER Gábor wrote:
> + cp -v "$MSG" "$GIT_DIR"/SQUASH_MSG
Sorry, but "cp -v" is not portable. It's not in POSIX, and this breaks
the script for (at least) Solaris.
However, it's not even clear to me why "-v" is used at all, considering
that the "squash" case above does not use it. Is it a debugging
leftover? Am I missing something?
[Aside: My Solaris 8 autobuilder is now running, which was a huge pain.
However, it is very satisfying to catch things like this in "next"
before they hit a wider audience.]
-Peff
^ permalink raw reply
* Re: [PATCHv4] gitweb: parse project/action/hash_base:filename PATH_INFO
From: Giuseppe Bilotta @ 2008-10-03 6:04 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Petr Baudis, Junio C Hamano, Shawn O. Pearce
In-Reply-To: <200810030248.57144.jnareb@gmail.com>
On Fri, Oct 3, 2008 at 2:48 AM, Jakub Narebski <jnareb@gmail.com> wrote:
> On Thu, 2 Oct 2008, Giuseppe Bilotta wrote:
>> OTOH, while setting both $hash and $hash_base has worked fine for me
>> so far (because the right one is automatically used and apparently
>> setting the other doesn't hurt), choosing which one to set is a much
>> hairier case. Do you have suggestions for a better way to always make
>> it work?
>
> Well, it is either checking $action and setting either $hash or
> $hash_base, or setting both, with some comments on why and when it is
> needed (as discussed on #git). IIUC $hash_base is needed only for
> filename-taking tree actions which acts on top-tree, and therefore
> don't need $file_name, like 'project/tree/branch' or related
> 'project/history/branch' (the latter is practically almost equivalent
> to 'project/shortlog/branch' or 'project/branch').
>
> I'm not sure if it wouldn't be better to call validate_refname($refname)
> once, either as:
>
> $hash_base ||= $hash ||= validate_refname($refname);
>
> but that might be incorrect in the obscure case of setting $hash via 'h'
> CGI query parameter, and letting gitweb to set-up $hash_base via
> path_info, so perhaps ($refname is local to evaluate_path_info, IIRC)
>
> $refname = validate_refname($refname);
> $hash ||= $refname;
> $hash_base ||= $refname;
I'll go with this.
> But that is just nitpicking this fragment of code to death. In short:
> either check which of $hash and $hash_base to set in this branch of
> conditional, or explain why setting both $hash and $hash_base is needed,
> and why it is acceptable, either as comments, or in commit message.
Comment is probably better, as long as I remember to move it with the
code it belongs to ;)
>>>> @@ -631,8 +642,15 @@ sub href (%) {
>>>> if ($params{-replay}) {
>>>> while (my ($name, $symbol) = each %mapping) {
>>>> if (!exists $params{$name}) {
>>>> - # to allow for multivalued params we use arrayref form
>>>> - $params{$name} = [ $cgi->param($symbol) ];
>>>> + # the parameter we want to recycle may be either part of the
>>>> + # list of CGI parameter, or recovered from PATH_INFO
>>>> + if ($cgi->param($symbol)) {
>>>> + # to allow for multivalued params we use arrayref form
>>>> + $params{$name} = [ $cgi->param($symbol) ];
>>>> + } else {
>>>> + no strict 'refs';
>>>> + $params{$name} = $$name if $$name;
>>>
>>> I would _perhaps_ add here comment that multivalued parameters can come
>>> only from CGI query string, so there is no need for something like:
>>>
>>> + $params{$name} = (ref($$name) ? @$name : $$name) if $$name;
>>>
>>>> + }
>>>> }
>>>> }
>>>> }
>>>
>>> This fragment is a bit of ugly code, hopefully corrected in later patch.
>>> I think it would be better to have 'refactor parsing/validation of input
>>> parameters' to be very fist patch in series; I am not sure but I suspect
>>> that is a kind of bugfix for current "$project/$hash" ('shortlog' view)
>>> and "$project/$hash_base:$file_name" ('blob_plain' and 'tree' view)
>>> path_info.
>>
>> But implementing the path_info parsing first makes the input param
>> refactoring SO much nicer that I would rather put a comment here
>> saying "this code sucks: we should rather collect all input
>> parameters" and then clean it up on the subsequent patch.
>
> Why not cleanup first?
Because cleaning it up depends on the refactoring, and the refactoring
is much cleaner when path_info already handles $action too.
> When implementing href(..., -replay=>1) I have forgot that some of
> gitweb parameters are implicitly passed ($project, because it is needed
> in most gitweb links), and some can be passed via path_info ($hash
> and/or $hash_base, $file_name). Your code adds $action to the mix, but
> it doesn't change the fact that 1.) even before your code -replay case
> was incorrect for some path_info links (handcrafted, as gitweb generates
> only $project via path_info); 2.) code you have added is a bit ugly.
>
> Besides using variables change a little meaning of -replay, namely
> in your code gitweb always sets action, even for non-path_name links
> when we started from "default action" (i.e. without action set) links.
> I guess this is mainly theoretical issue, as I don't think that default
> views use many -replay links.
Ah the issue of the default action is something I hadn't taken into
consideration, actually. Now the question is, should replay keep
default -> default, or should it go with default -> last incantation?
> P.S. with the idea of pushing parameters obtained not from CGI query
> string to $cgi->param() via "$cgi->param($name, $value);" or in named
> params form "$cgi->(-name=>$name, -value=>$value);" you would not need
> to change (a bit hacky, admittedly) href(...,-replay=>1) code.
Yes, but it would muddy the waters about 'where did this parameter
come from' in case we ever need to know that.
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* [PATCH] Makefile: do not set NEEDS_LIBICONV for Solaris 8
From: Jeff King @ 2008-10-03 6:39 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Brandon Casey, git
This breaks my build on Solaris 8, as there is no separate
libiconv.
The history of this line is somewhat convoluted. In 2fd955c
(in November 2005), NEEDS_LIBICONV was turned on for all
Solaris builds, claiming to "fix an error in Solaris 10 by
setting NEEDS_LIBICONV".
Later, e15f545 (in February of 2006) claimed that "Solaris
9+ don't need iconv", and moved NEEDS_LIBICONV into a
section for Solaris 8.
Furthermore, Brandon Casey claims in
<5A1KxlhmUIHe8iXPxnXYuNXsq0Yjlbwkz2eBin3z7ELuL9nK-4tSpw@cipher.nrlssc.navy.mil>
that he does not set NEEDS_LIBICONV for Solaris 7.
So either one of those commits is totally wrong, or there is
some other magic going on where some Solaris installs need
it and others don't.
Given Brandon's statement and my problems on Solaris 8 with
NEEDS_LIBICONV, I am inclined to think the first commit was
bogus, and that NEEDS_LIBICONV shouldn't be set for Solaris
at all by default. If somebody wants to use iconv and has
installed it manually, they can set it in their config.mak.
Signed-off-by: Jeff King <peff@peff.net>
---
Makefile | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index 3abbb4e..308dc70 100644
--- a/Makefile
+++ b/Makefile
@@ -651,7 +651,6 @@ ifeq ($(uname_S),SunOS)
NO_MKDTEMP = YesPlease
OLD_ICONV = UnfortunatelyYes
ifeq ($(uname_R),5.8)
- NEEDS_LIBICONV = YesPlease
NO_UNSETENV = YesPlease
NO_SETENV = YesPlease
NO_C99_FORMAT = YesPlease
--
1.6.0.2.624.g13e35
^ permalink raw reply related
* Re: [PATCHv4] gitweb: refactor input parameters parse/validation
From: Giuseppe Bilotta @ 2008-10-03 7:24 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Petr Baudis, Junio C Hamano, Shawn O. Pearce
In-Reply-To: <200810030336.31469.jnareb@gmail.com>
On Fri, Oct 3, 2008 at 3:36 AM, Jakub Narebski <jnareb@gmail.com> wrote:
> On Thu, 2 Oct 2008, Giuseppe Bilotta wrote:
>
>> Since input parameters can now be obtained both from CGI parameters and
>> PATH_INFO, we would like most of the code to be agnostic about the way
>> parameters were retrieved.
>
> I'd prefer that this cleanup/refactoring patch was _first_ patch in
> the series, as we were able to obtain parameters both from CGI query
> parameters and from PATH_INFO ($project, $hash or $hash_base+$file_name)
> _before_ first patch in this series. So it correct not only issue
> introduced by first patch (and fixed somewhat there), but what was
> outstanding (but rare because gitweb did not generate such links)
> issue.
It's true that the patch makes sense regardless of the rest of the
path_info patch, indeed.
>> We thus collect all the parameters into the new %input_params hash,
>> removing the need for ad-hoc kludgy code in href().
>
> Alternate solution would be push data from PATH_INFO in query params
> data using (for example)
>
> $cgi->param('a', $action);
>
> or, naming parameters explicitely
>
> $cgi->param(-name=>'a', -value=>$action);
>
> This avoids need for additional variable, reuses current code, and
> nicely sidesteps issue whether to use long names for keys ('action',
> 'file_name') or short ones from CGI query ('a', 'f').
>
>
> It would probably has to be at least partially written to check which
> of those two solutions (%input_params or $cgi->param('a', $a)))
> is better.
If we really don't want to care where the parameters came from *at
all*, writing on $cgi->param is likely to be the cleanest solution.
>> As much of the
>> parameters validation code is now shared between CGI and PATH_INFO,
>> although this requires PATH_INFO parsing to be interleaved into the main
>> code instead of being factored out into its own routine.
>
> I'm not sure if it is worth it then to unify parameters validation,
> with such drawback.
Yeah, I don't like it very much either. But it does spare a little on
the validation. OTOH, the amount we spare is not extraordinary, so
it's probably not worth the spaghettization of the code ...
>> # if we're called with PATH_INFO, we have to strip that
>> -# from the URL to find our real URL
>> -if (my $path_info = $ENV{"PATH_INFO"}) {
>> +# from the URL to find our real URL. PATH_INFO is kept
>> +# as it's used later on for parameter extraction
>> +my $path_info = $ENV{"PATH_INFO"};
>> +if ($path_info) {
>> $my_url =~ s,\Q$path_info\E$,,;
>> $my_uri =~ s,\Q$path_info\E$,,;
>> }
>
> This might be separate patch, if you wanted to increase your commit
> count ;-) More seriously I think it should be at least briefly
> mentioned in commit message (make $path_info global).
Note however that the path_info evaluation is _destructive_, so after
evaluation is complete we don't have much of it left.
[snip]
> Nice, although I'm not sure if [%@]cgi_param_mapping has to global.
> If we use long parameters names as keys, I think it has to, somewhat.
> See also comment below.
>> +# fill %input_params with the CGI parameters. All values except for 'opt'
>> +# should be single values, but opt can be an array. We should probably
>> +# build an array of parameters that can be multi-valued, but since for the time
>> +# being it's only this one, we just single it out
>> +while (my ($name, $symbol) = each %cgi_param_mapping) {
>> + if ($symbol eq 'opt') {
>> + $input_params{$name} = [$cgi->param($symbol)];
>> + } else {
>> + $input_params{$name} = $cgi->param($symbol);
>> }
>> }
>
> If it was chosen to use short (CGI query) parameter names, the above
> loop could be replaced by simple
>
> %input_params = $cgi->Vars;
>
> or to be more exact, if we want to have multi-valued parameters stored
> as array ref
>
> %input_params = map { [ split /\0/ ] if /\0/; } $cgi->Vars;
>
>
> See CGI(3pm):
>
> When using this [Vars], the thing you must watch out for are multivalued CGI
> parameters. Because a hash cannot distinguish between scalar and list con-
> text, multivalued parameters will be returned as a packed string, separated
> by the "\0" (null) character. You must split this packed string in order
> to get at the individual values.
Ah, an interesting alternative. This would make parameter copying a
one-liner, almost as good as just using $cgi->param for everything :)
>> -# parameters which are pathnames
>> -our $project = $cgi->param('p');
>> +# next, we want to parse PATH_INFO (which was already stored in $path_info at
>> +# the beginning). This is a little hairy because PATH_INFO parsing needs some
>> +# form of parameter validation, so we interleave parsing and validation.
>
> I don't think it is a good idea. In my opinion, for my taste, it would
> be better to separate evaluating path_info from the rest.
>
> We could instead introduce convention that if variable (like $project)
> is set, then it is assumed to be validated; if it is present only in
> the %input_params hash, then it has to be validated.
>
>
> On the other hand this ordering, first by parameter, then by method
> of extraction could be seem quite equally valid. Nevertheless I think
> that previous flow with separate evaluate_path_info() and what should
> be evaluate_CGI_query() has better encapsulation.
>
>> +#
>> +# The accepted PATH_INFO syntax is $project/$action/$hash or
>> +# $project/$action/$hash_base:$file_name, where $action may be missing (mostly for
>> +# backwards compatibility), so we need to parse and validate the parameters in
>> +# this same order.
>> +
>> +# clear $path_info of any leading /
>> +$path_info =~ s,^/+,,;
>> +
>> +our $project = $input_params{'project'};
>> +if ($path_info && !defined $project) {
>> + # if $project was not defined by CGI, we try to extract it from
>> + # $path_info
>> + $project = $path_info;
>> + $project =~ s,/+$,,;
>> + while ($project && !check_head_link("$projectroot/$project")) {
>> + $project =~ s,/*[^/]*$,,;
>> + }
>> + $input_params{'project'} = $project;
>> +} else {
>> + # otherwise, we suppress $path_info parsing altogether
>> + $path_info = undef;
>> +}
>> +
>> +# project name validation
>> if (defined $project) {
>> if (!validate_pathname($project) ||
>> !(-d "$projectroot/$project") ||
>
> Note that this code does less checking if $project is in path_info than
> for the case where it is set by CGI query. Perhaps there should be base
> fast check in a loop, and more extensive test later.
Uh ... isn't this exactly what's happening? In the loop we just gobble
until we find a git dir. Validation is then done, and it's the _same_
validation for both cases. Why do you say that path_info $project is
less checked?
>> @@ -408,16 +506,66 @@ if (defined $project) {
>> undef $project;
>> die_error(404, "No such project");
>> }
>> +
>> + # we purge the $project name from the $path_info, preparing it for
>> + # subsequent parameters extraction
>> + $path_info =~ s,^\Q$project\E/*,, if defined $path_info;
>> +} else {
>> + # we also suppress $path_info parsing if no project was defined
>> + $path_info = undef;
>> +}
>
> In evaluate_path_info it was simply 'return if...'; here with mentioned
> interleaving it is harder and a bit hacky.
I know.
>> $params{'project'} = $project unless exists $params{'project'};
>>
>> if ($params{-replay}) {
>> - while (my ($name, $symbol) = each %mapping) {
>> - if (!exists $params{$name}) {
>> - # the parameter we want to recycle may be either part of the
>> - # list of CGI parameter, or recovered from PATH_INFO
>> - if ($cgi->param($symbol)) {
>> - # to allow for multivalued params we use arrayref form
>> - $params{$name} = [ $cgi->param($symbol) ];
>> - } else {
>> - no strict 'refs';
>> - $params{$name} = $$name if $$name;
>> - }
>> - }
>> + while (my ($name, $val) = each %input_params) {
>> + $params{$name} = $input_params{$name}
>> + unless (exists $params{$name});
>
> Very nice, short code. Should be something like that from
> the very beginning.
Ok, I'll try working up a patch for params merging before any
path_info extensions.
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* [PATCH] gitweb: Support for simple project search form
From: Petr Baudis @ 2008-10-03 7:29 UTC (permalink / raw)
To: git, git; +Cc: Petr Baudis
In-Reply-To: <1222960621-12044-1-git-send-email-pasky@suse.cz>
This is a trivial patch adding support for searching projects by name
and description, making use of the "infrastructure" provided by the
tag cloud generation.
Signed-off-by: Petr Baudis <petr.baudis@novartis.com>
---
gitweb/gitweb.css | 4 ++++
gitweb/gitweb.perl | 12 ++++++++++--
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index 07f5b53..a01eac8 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -435,6 +435,10 @@ div.search {
right: 12px
}
+p.projsearch {
+ text-align: center;
+}
+
td.linenr {
text-align: right;
}
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 07fa1e6..4bc8a12 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3758,11 +3758,14 @@ sub git_project_list_body {
my $pr = $projects[$i];
next if $tagfilter and $show_ctags and not grep { lc $_ eq lc $tagfilter } keys %{$pr->{'ctags'}};
- # Weed out forks
+ next if $searchtext and not $pr->{'path'} =~ /$searchtext/
+ and not $pr->{'descr_long'} =~ /$searchtext/;
+ # Weed out forks or non-matching entries of search
if ($check_forks) {
my $forkbase = $project; $forkbase ||= ''; $forkbase =~ s#\.git$#/#;
$forkbase="^$forkbase" if $forkbase;
- next if not $tagfilter and $pr->{'path'} =~ m#$forkbase.*/.*#; # regexp-safe
+ next if not $searchtext and not $tagfilter and $show_ctags
+ and $pr->{'path'} =~ m#$forkbase.*/.*#; # regexp-safe
}
if ($alternate) {
@@ -4099,6 +4102,11 @@ sub git_project_list {
close $fd;
print "</div>\n";
}
+ print $cgi->startform(-method => "get") .
+ "<p class=\"projsearch\">Search:\n" .
+ $cgi->textfield(-name => "s", -value => $searchtext) . "\n" .
+ "</p>" .
+ $cgi->end_form() . "\n";
git_project_list_body(\@list, $order);
git_footer_html();
}
--
tg: (0d352a0..) t/misc/projsearch (depends on: t/tagcloud/forks)
^ permalink raw reply related
* [PATCH] git-gui: Add a search command to the blame viewer.
From: Alexander Gavrilov @ 2008-10-03 7:36 UTC (permalink / raw)
To: git; +Cc: Shawn O. Pearce
In-Reply-To: <1223019414-24643-1-git-send-email-angavrilov@gmail.com>
One of the largest deficiencies in the blame viewer at
the moment is the impossibility to search for a text
string. This commit fixes it by adding a Firefox-like
search panel to the viewer.
The panel can be shown by pressing F7 or clicking a
menu entry, and is hidden by pressing Esc. Find Next
is available through the F3 key.
Implementation is based on the gitk code, but heavily
refactored. It now also supports case-insensitive
searches, and uses the text box background color to
signal success or failure of the search.
Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
I would have used Ctrl-F, but it is already occupied.
-- Alexander
git-gui.sh | 3 +
lib/blame.tcl | 37 ++++++++++-
lib/search.tcl | 190 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 227 insertions(+), 3 deletions(-)
create mode 100644 lib/search.tcl
diff --git a/git-gui.sh b/git-gui.sh
index 79a108d..4f95139 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -591,6 +591,7 @@ bind . <Visibility> {
if {[is_Windows]} {
wm iconbitmap . -default $oguilib/git-gui.ico
+ set ::tk::AlwaysShowSelection 1
}
######################################################################
@@ -1067,6 +1068,8 @@ set selected_commit_type new
set nullid "0000000000000000000000000000000000000000"
set nullid2 "0000000000000000000000000000000000000001"
+set have_tk85 [expr {[package vcompare $tk_version "8.5"] >= 0}]
+
######################################################################
##
## task management
diff --git a/lib/blame.tcl b/lib/blame.tcl
index 221313c..a45784c 100644
--- a/lib/blame.tcl
+++ b/lib/blame.tcl
@@ -21,9 +21,11 @@ field w_amov ; # text column: annotations + move tracking
field w_asim ; # text column: annotations (simple computation)
field w_file ; # text column: actual file data
field w_cviewer ; # pane showing commit message
+field finder ; # find mini-dialog frame
field status ; # status mega-widget instance
field old_height ; # last known height of $w.file_pane
+
# Tk UI colors
#
variable active_color #c0edc5
@@ -59,7 +61,7 @@ field tooltip_timer {} ; # Current timer event for our tooltip
field tooltip_commit {} ; # Commit(s) in tooltip
constructor new {i_commit i_path i_jump} {
- global cursor_ptr
+ global cursor_ptr M1B M1T have_tk85
variable active_color
variable group_colors
@@ -199,6 +201,11 @@ constructor new {i_commit i_path i_jump} {
-width 80 \
-xscrollcommand [list $w.file_pane.out.sbx set] \
-font font_diff
+ if {$have_tk85} {
+ $w_file configure -inactiveselectbackground darkblue
+ }
+ $w_file tag conf found \
+ -background yellow
set w_columns [list $w_amov $w_asim $w_line $w_file]
@@ -219,6 +226,11 @@ constructor new {i_commit i_path i_jump} {
-weight 1
grid rowconfigure $w.file_pane.out 0 -weight 1
+ set finder [::searchbar::new \
+ $w.file_pane.out.ff $w_file \
+ -column [expr {[llength $w_columns] - 1}] \
+ ]
+
set w_cviewer $w.file_pane.cm.t
text $w_cviewer \
-background white \
@@ -259,6 +271,10 @@ constructor new {i_commit i_path i_jump} {
-label [mc "Copy Commit"] \
-command [cb _copycommit]
$w.ctxm add separator
+ $w.ctxm add command \
+ -label [mc "Find Text..."] \
+ -accelerator F7 \
+ -command [list searchbar::show $finder]
menu $w.ctxm.enc
build_encoding_menu $w.ctxm.enc [cb _setencoding]
$w.ctxm add cascade \
@@ -280,9 +296,15 @@ constructor new {i_commit i_path i_jump} {
$i tag conf color$g -background [lindex $group_colors $g]
}
+ if {$i eq $w_file} {
+ $w_file tag raise found
+ }
+ $i tag raise sel
+
$i conf -cursor $cursor_ptr
- $i conf -yscrollcommand [list many2scrollbar \
- $w_columns yview $w.file_pane.out.sby]
+ $i conf -yscrollcommand \
+ "[list ::searchbar::scrolled $finder]
+ [list many2scrollbar $w_columns yview $w.file_pane.out.sby]"
bind $i <Button-1> "
[cb _hide_tooltip]
[cb _click $i @%x,%y]
@@ -319,6 +341,11 @@ constructor new {i_commit i_path i_jump} {
bind $w_cviewer <Tab> "[list focus $w_file];break"
bind $w_cviewer <Button-1> [list focus $w_cviewer]
bind $w_file <Visibility> [list focus $w_file]
+ bind $top <F7> [list searchbar::show $finder]
+ bind $top <Escape> [list searchbar::hide $finder]
+ bind $top <F3> [list searchbar::find_next $finder]
+ bind $top <Shift-F3> [list searchbar::find_prev $finder]
+ catch { bind $top <Shift-Key-XF86_Switch_VT_3> [list searchbar::find_prev $finder] }
grid configure $w.header -sticky ew
grid configure $w.file_pane -sticky nsew
@@ -873,6 +900,10 @@ method _showcommit {cur_w lno} {
foreach i $w_columns {
$i tag conf g$cmit -background $active_color
$i tag raise g$cmit
+ if {$i eq $w_file} {
+ $w_file tag raise found
+ }
+ $i tag raise sel
}
set author_name {}
diff --git a/lib/search.tcl b/lib/search.tcl
new file mode 100644
index 0000000..d292f20
--- /dev/null
+++ b/lib/search.tcl
@@ -0,0 +1,190 @@
+# incremental search panel
+# based on code from gitk, Copyright (C) Paul Mackerras
+
+class searchbar {
+
+field w
+field ctext
+
+field searchstring {}
+field casesensitive 1
+field searchdirn -forwards
+
+field smarktop
+field smarkbot
+
+constructor new {i_w i_text args} {
+ set w $i_w
+ set ctext $i_text
+
+ frame $w
+ label $w.l -text [mc Find:]
+ button $w.bn -text [mc Next] -command [cb find_next]
+ button $w.bp -text [mc Prev] -command [cb find_prev]
+ checkbutton $w.cs -text [mc Case-Sensitive] \
+ -variable ${__this}::casesensitive -command [cb _incrsearch]
+ entry $w.ent -textvariable ${__this}::searchstring -background lightgreen
+ pack $w.l -side left
+ pack $w.cs -side right
+ pack $w.bp -side right
+ pack $w.bn -side right
+ pack $w.ent -side left -expand 1 -fill x
+
+ eval grid conf $w -sticky we $args
+ grid remove $w
+
+ trace add variable searchstring write [cb _incrsearch_cb]
+
+ bind $w <Destroy> [cb delete_this]
+ return $this
+}
+
+method show {} {
+ if {![winfo ismapped $w]} {
+ grid $w
+ }
+ focus -force $w.ent
+}
+
+method hide {} {
+ if {[winfo ismapped $w]} {
+ focus $ctext
+ grid remove $w
+ }
+}
+
+method _get_new_anchor {} {
+ # use start of selection if it is visible,
+ # or the bounds of the visible area
+ set top [$ctext index @0,0]
+ set bottom [$ctext index @0,[winfo height $ctext]]
+ set sel [$ctext tag ranges sel]
+ if {$sel ne {}} {
+ set spos [lindex $sel 0]
+ if {[lindex $spos 0] >= [lindex $top 0] &&
+ [lindex $spos 0] <= [lindex $bottom 0]} {
+ return $spos
+ }
+ }
+ if {$searchdirn eq "-forwards"} {
+ return $top
+ } else {
+ return $bottom
+ }
+}
+
+method _get_wrap_anchor {dir} {
+ if {$dir eq "-forwards"} {
+ return 1.0
+ } else {
+ return end
+ }
+}
+
+method _do_search {start {mlenvar {}} {dir {}} {endbound {}}} {
+ set cmd [list $ctext search]
+ if {$mlenvar ne {}} {
+ upvar $mlenvar mlen
+ lappend cmd -count mlen
+ }
+ if {!$casesensitive} {
+ lappend cmd -nocase
+ }
+ if {$dir eq {}} {
+ set dir $searchdirn
+ }
+ lappend cmd $dir -- $searchstring
+ if {$endbound ne {}} {
+ set here [eval $cmd [list $start] [list $endbound]]
+ } else {
+ set here [eval $cmd [list $start]]
+ if {$here eq {}} {
+ set here [eval $cmd [_get_wrap_anchor $this $dir]]
+ }
+ }
+ return $here
+}
+
+method _incrsearch_cb {name ix op} {
+ after idle [cb _incrsearch]
+}
+
+method _incrsearch {} {
+ $ctext tag remove found 1.0 end
+ if {[catch {$ctext index anchor}]} {
+ $ctext mark set anchor [_get_new_anchor $this]
+ }
+ if {$searchstring ne {}} {
+ set here [_do_search $this anchor mlen]
+ if {$here ne {}} {
+ $ctext see $here
+ $ctext tag remove sel 1.0 end
+ $ctext tag add sel $here "$here + $mlen c"
+ $w.ent configure -background lightgreen
+ _set_marks $this 1
+ } else {
+ $w.ent configure -background lightpink
+ }
+ }
+}
+
+method find_prev {} {
+ find_next $this -backwards
+}
+
+method find_next {{dir -forwards}} {
+ focus $w.ent
+ $w.ent icursor end
+ set searchdirn $dir
+ $ctext mark unset anchor
+ if {$searchstring ne {}} {
+ set start [_get_new_anchor $this]
+ if {$dir eq "-forwards"} {
+ set start "$start + 1c"
+ }
+ set match [_do_search $this $start mlen]
+ $ctext tag remove sel 1.0 end
+ if {$match ne {}} {
+ $ctext see $match
+ $ctext tag add sel $match "$match + $mlen c"
+ }
+ }
+}
+
+method _mark_range {first last} {
+ set mend $first.0
+ while {1} {
+ set match [_do_search $this $mend mlen -forwards $last.end]
+ if {$match eq {}} break
+ set mend "$match + $mlen c"
+ $ctext tag add found $match $mend
+ }
+}
+
+method _set_marks {doall} {
+ set topline [lindex [split [$ctext index @0,0] .] 0]
+ set botline [lindex [split [$ctext index @0,[winfo height $ctext]] .] 0]
+ if {$doall || $botline < $smarktop || $topline > $smarkbot} {
+ # no overlap with previous
+ _mark_range $this $topline $botline
+ set smarktop $topline
+ set smarkbot $botline
+ } else {
+ if {$topline < $smarktop} {
+ _mark_range $this $topline [expr {$smarktop-1}]
+ set smarktop $topline
+ }
+ if {$botline > $smarkbot} {
+ _mark_range $this [expr {$smarkbot+1}] $botline
+ set smarkbot $botline
+ }
+ }
+}
+
+method scrolled {} {
+ if {$searchstring ne {}} {
+ after idle [cb _set_marks 0]
+ }
+}
+
+}
\ No newline at end of file
--
1.6.0.20.g6148bc
^ permalink raw reply related
* [PATCH] git-gui: Fix the blame window shape.
From: Alexander Gavrilov @ 2008-10-03 7:36 UTC (permalink / raw)
To: git; +Cc: Shawn O. Pearce
On modern high-resolution monitors the blame viewer
window is very high, yet too narrow. This patch
makes it gravitate to a more sane resolution, which
takes the font size into account.
It also changes the default text view size to 80% of
the window, and slightly modifies the border decorations
for better appearance.
Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
lib/blame.tcl | 19 +++++++++++++------
1 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/lib/blame.tcl b/lib/blame.tcl
index eb61374..221313c 100644
--- a/lib/blame.tcl
+++ b/lib/blame.tcl
@@ -69,6 +69,8 @@ constructor new {i_commit i_path i_jump} {
make_toplevel top w
wm title $top [append "[appname] ([reponame]): " [mc "File Viewer"]]
+ set font_w [font measure font_diff "0"]
+
frame $w.header -background gold
label $w.header.commit_l \
-text [mc "Commit:"] \
@@ -114,9 +116,9 @@ constructor new {i_commit i_path i_jump} {
pack $w_path -fill x -side right
pack $w.header.path_l -side right
- panedwindow $w.file_pane -orient vertical
- frame $w.file_pane.out
- frame $w.file_pane.cm
+ panedwindow $w.file_pane -orient vertical -borderwidth 0 -sashwidth 3
+ frame $w.file_pane.out -relief flat -borderwidth 1
+ frame $w.file_pane.cm -relief sunken -borderwidth 1
$w.file_pane add $w.file_pane.out \
-sticky nsew \
-minsize 100 \
@@ -328,9 +330,14 @@ constructor new {i_commit i_path i_jump} {
set req_w [winfo reqwidth $top]
set req_h [winfo reqheight $top]
- set scr_h [expr {[winfo screenheight $top] - 100}]
- if {$req_w < 600} {set req_w 600}
+ set scr_w [expr {[winfo screenwidth $top] - 40}]
+ set scr_h [expr {[winfo screenheight $top] - 120}]
+ set opt_w [expr {$font_w * (80 + 5*3 + 3)}]
+ if {$req_w < $opt_w} {set req_w $opt_w}
+ if {$req_w > $scr_w} {set req_w $scr_w}
+ set opt_h [expr {$req_w*4/3}]
if {$req_h < $scr_h} {set req_h $scr_h}
+ if {$req_h > $opt_h} {set req_h $opt_h}
set g "${req_w}x${req_h}"
wm geometry $top $g
update
@@ -338,7 +345,7 @@ constructor new {i_commit i_path i_jump} {
set old_height [winfo height $w.file_pane]
$w.file_pane sash place 0 \
[lindex [$w.file_pane sash coord 0] 0] \
- [expr {int($old_height * 0.70)}]
+ [expr {int($old_height * 0.80)}]
bind $w.file_pane <Configure> \
"if {{$w.file_pane} eq {%W}} {[cb _resize %h]}"
--
1.6.0.20.g6148bc
^ permalink raw reply related
* [PATCH] git-gui: Fix the blame viewer destroy handler.
From: Alexander Gavrilov @ 2008-10-03 7:36 UTC (permalink / raw)
To: git; +Cc: Shawn O. Pearce
In-Reply-To: <1223019414-24643-1-git-send-email-angavrilov@gmail.com>
It did not delete the object, which is not very good.
Also, destroy may be fired up for subwindows, so we
should check %W.
Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
My own bug. I hope that now I understand
Tcl better than 2 months ago.
-- Alexander
lib/blame.tcl | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/lib/blame.tcl b/lib/blame.tcl
index a45784c..765d08c 100644
--- a/lib/blame.tcl
+++ b/lib/blame.tcl
@@ -377,11 +377,18 @@ constructor new {i_commit i_path i_jump} {
"if {{$w.file_pane} eq {%W}} {[cb _resize %h]}"
wm protocol $top WM_DELETE_WINDOW "destroy $top"
- bind $top <Destroy> [cb _kill]
+ bind $top <Destroy> [cb _handle_destroy %W]
_load $this $i_jump
}
+method _handle_destroy {win} {
+ if {$win eq $w} {
+ _kill $this
+ delete_this
+ }
+}
+
method _kill {} {
if {$current_fd ne {}} {
kill_file_process $current_fd
--
1.6.0.20.g6148bc
^ permalink raw reply related
* [PATCH] git-gui: Show a round number of bytes of large untracked text files
From: Johannes Sixt @ 2008-10-03 8:28 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Git Mailing List
From: Johannes Sixt <johannes.sixt@telecom.at>
If an untracked text file is selected, then its contents are displayed
instead of a diff. If the file is large, then the following hint is
inserted at the top:
* Untracked file is 14774881 bytes.
* Showing only first 131072 bytes.
Why exactly 131072 bytes? With this patch it is 100000 bytes.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
Another bikeshed to paint. ;)
Every now and then I stumble over this number, and I ask myself,
why are those h4x3rs so binary?
-- Hannes
lib/diff.tcl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/lib/diff.tcl b/lib/diff.tcl
index abe502d..6e08704 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -164,7 +164,7 @@ proc show_other_diff {path w m cont_info} {
# - Git won't give us the diff, there's nothing to compare to!
#
if {$m eq {_O}} {
- set max_sz [expr {128 * 1024}]
+ set max_sz 100000
set type unknown
if {[catch {
set type [file type $path]
--
1.6.0.2.319.gffa7c
^ permalink raw reply related
* Re: [PATCH] git-gui: Add a search command to the blame viewer.
From: Andreas Ericsson @ 2008-10-03 8:29 UTC (permalink / raw)
To: Alexander Gavrilov; +Cc: git, Shawn O. Pearce
In-Reply-To: <1223019414-24643-2-git-send-email-angavrilov@gmail.com>
Alexander Gavrilov wrote:
> One of the largest deficiencies in the blame viewer at
> the moment is the impossibility to search for a text
> string. This commit fixes it by adding a Firefox-like
> search panel to the viewer.
>
> The panel can be shown by pressing F7 or clicking a
> menu entry, and is hidden by pressing Esc. Find Next
> is available through the F3 key.
>
> Implementation is based on the gitk code, but heavily
> refactored. It now also supports case-insensitive
> searches, and uses the text box background color to
> signal success or failure of the search.
>
> Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
> ---
>
What with me being a total tcl nubling, I can't comment on the
patch, but the intentions are clearly full of win and awesome.
I was utterly surprised to notice this functionality wasn't in
there the first time I needed it.
Thanks.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* rebase -i: remove leftover debugging
From: SZEDER Gábor @ 2008-10-03 9:33 UTC (permalink / raw)
To: Jeff King, Shawn O. Pearce; +Cc: git
In-Reply-To: <20081003060110.GA4473@coredump.intra.peff.net>
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
On Fri, Oct 03, 2008 at 02:01:10AM -0400, Jeff King wrote:
> Sorry, but "cp -v" is not portable. It's not in POSIX, and this breaks
> the script for (at least) Solaris.
>
> However, it's not even clear to me why "-v" is used at all, considering
> that the "squash" case above does not use it. Is it a debugging
> leftover? Am I missing something?
Oh, yeah, it's just leftover debugging. Thanks for catching.
git-rebase--interactive.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 77e1132..ec4299a 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -322,7 +322,7 @@ do_next () {
MSG_OPT=
MSG_FILE=
rm -f "$SQUASH_MSG" || exit
- cp -v "$MSG" "$GIT_DIR"/SQUASH_MSG
+ cp "$MSG" "$GIT_DIR"/SQUASH_MSG
rm -f "$GIT_DIR"/MERGE_MSG || exit
;;
esac
--
1.6.0.2.430.gfc53
^ permalink raw reply related
* Re: [ANNOUNCE] TopGit v0.3
From: Jan Holesovsky @ 2008-10-03 10:00 UTC (permalink / raw)
To: Petr Baudis; +Cc: Jan Nieuwenhuizen, martin f krafft, git
In-Reply-To: <20080923132728.GV10360@machine.or.cz>
Hi Pasky,
On Tuesday 23 September 2008 15:27, Petr Baudis wrote:
> But if you scenario indeed is totally generic, I'm afraid I don't know
> how to make TopGit remove dependencies, except perhaps for the price of
> massive complexity and massive slowdown (pretty much redoing all the
> history walking etc.). Maybe someone else comes by with a genial
> solution...
Still thinking about this, and I think we [ooo-builders ;-)] could live with
the ugliest of the ugly way of doing this: When you have a topgit branch
t/b1, and would like to undepend it, just
- tg patch t/b1 > save.diff
- commit a reverse of save.diff to t/b1
- tg update everything
- remove t/b1 from all the .topdeps
- commit save.diff to t/b1 again ;-)
Yeah, it creates 2 more commits in t/b1, but that's bearable I think [we do
disable patches, but not every day ;-)], you still have the history, and you
are able to add the dependency later again.
What do you think, please?
Regards,
Jan
^ permalink raw reply
* [PATCH] git-gui: Correctly set up locators in case of preset URL variable
From: Petr Baudis @ 2008-10-03 10:13 UTC (permalink / raw)
To: git, git; +Cc: spearce, Petr Baudis
In-Reply-To: <20080930195839.GK21310@spearce.org>
This patch fixes locators setup in case the URL variable is already set,
e.g. in the clone dialog during 'git gui clone'.
Signed-off-by: Petr Baudis <petr.baudis@novartis.com>
---
git-gui/lib/transport.tcl | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/git-gui/lib/transport.tcl b/git-gui/lib/transport.tcl
index 277e6b8..02c4eca 100644
--- a/git-gui/lib/transport.tcl
+++ b/git-gui/lib/transport.tcl
@@ -68,8 +68,13 @@ proc location_input {widget urlvar op} {
global _locator_template _locator_input _locator_var
trace remove variable _locator_input write locator_update
- set _locator_template $default_locator
- set _locator_input {}
+ if {[set $urlvar] == {}} {
+ set _locator_template $default_locator
+ set _locator_input {}
+ } else {
+ set _locator_template "URL"
+ set _locator_input [set $urlvar]
+ }
set _locator_var $urlvar
trace add variable _locator_input write locator_update
--
tg: (3c6c738..) t/git-gui/locator-preset (depends on: git-gui/locators t/git-gui/clonecmd)
^ permalink raw reply related
* Re: [PATCH] git-gui: Implement a 'clone' subcommand
From: Petr Baudis @ 2008-10-03 10:17 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Petr Baudis
In-Reply-To: <20080930195330.GA4413@spearce.org>
On Tue, Sep 30, 2008 at 12:53:30PM -0700, Shawn O. Pearce wrote:
> Petr Baudis <pasky@suse.cz> wrote:
> > Documentation/git-gui.txt | 5 +++++
> > git-gui/git-gui.sh | 21 ++++++++++++++++++---
> > git-gui/lib/choose_repository.tcl | 11 ++++++++++-
> > 3 files changed, 33 insertions(+), 4 deletions(-)
>
> This looks fine, except for the diffstat. You can't patch
> the docs and the code in the same patch as they are in two
> different repositories... :-|
Oh, I forgot again, sorry. :-( But I think this shows rather bad design
of the layout, doesn't it? Shouldn't this documentation be kept in
git-gui/Documentation/{git-gui,config}.txt and included from git's
Documentation/?
> Also, I don't know if you've noticed but I think tg is
> sending duplicate "To" headers in the messages:
Thanks - I have noticed already but I didn't have time yet to fix it and
it seems generally pretty harmless. I will try to do something with that
later.
--
Petr "Pasky" Baudis
People who take cold baths never have rheumatism, but they have
cold baths.
^ permalink raw reply
* Re: [PATCH] Do not rename read-only files during a push
From: Petr Baudis @ 2008-10-03 10:20 UTC (permalink / raw)
To: git; +Cc: spearce
In-Reply-To: <1222104029-28366-1-git-send-email-pasky@suse.cz>
On Mon, Sep 22, 2008 at 07:20:29PM +0200, Petr Baudis wrote:
> Win32 does not allow renaming read-only files (at least on a Samba
> share), making push into a local directory to fail. Thus, defer
> the chmod() call in index-pack.c:final() only after
> move_temp_to_file() was called.
>
> Signed-off-by: Petr Baudis <pasky@suse.cz>
Ping?
Petr "Pasky" Baudis
^ permalink raw reply
* Re: [PATCH] git-gui: Correctly set up locators in case of preset URL variable
From: H.Merijn Brand @ 2008-10-03 10:23 UTC (permalink / raw)
To: Petr Baudis; +Cc: git, spearce, Petr Baudis
In-Reply-To: <1223028826-10306-1-git-send-email-pasky@suse.cz>
On Fri, 3 Oct 2008 12:13:46 +0200, Petr Baudis <pasky@suse.cz> wrote:
> This patch fixes locators setup in case the URL variable is already set,
> e.g. in the clone dialog during 'git gui clone'.
While you are working on git-gui's transport.tcl, would you consider a
user setting to set the default for pushing tags to remote repositories?
After installation/update I always `fix' that by hand now.
> Signed-off-by: Petr Baudis <petr.baudis@novartis.com>
>
> ---
> git-gui/lib/transport.tcl | 9 +++++++--
> 1 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/git-gui/lib/transport.tcl b/git-gui/lib/transport.tcl
> index 277e6b8..02c4eca 100644
> --- a/git-gui/lib/transport.tcl
> +++ b/git-gui/lib/transport.tcl
> @@ -68,8 +68,13 @@ proc location_input {widget urlvar op} {
>
> global _locator_template _locator_input _locator_var
> trace remove variable _locator_input write locator_update
> - set _locator_template $default_locator
> - set _locator_input {}
> + if {[set $urlvar] == {}} {
> + set _locator_template $default_locator
> + set _locator_input {}
> + } else {
> + set _locator_template "URL"
> + set _locator_input [set $urlvar]
> + }
> set _locator_var $urlvar
> trace add variable _locator_input write locator_update
--
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: [PATCH] git-gui: Correctly set up locators in case of preset URL variable
From: Petr Baudis @ 2008-10-03 10:48 UTC (permalink / raw)
To: H.Merijn Brand; +Cc: git, spearce, Petr Baudis
In-Reply-To: <20081003122329.432c6359@pc09.procura.nl>
On Fri, Oct 03, 2008 at 12:23:29PM +0200, H.Merijn Brand wrote:
> On Fri, 3 Oct 2008 12:13:46 +0200, Petr Baudis <pasky@suse.cz> wrote:
>
> > This patch fixes locators setup in case the URL variable is already set,
> > e.g. in the clone dialog during 'git gui clone'.
>
> While you are working on git-gui's transport.tcl, would you consider a
> user setting to set the default for pushing tags to remote repositories?
>
> After installation/update I always `fix' that by hand now.
Actually, I think for the time being I'm finished with transport.tcl,
I'm sorry. :-) I probably won't work further on git-gui now that my
Novartis stay is almost over. My last stab will be probably trying to
change locators support to use insteadOf and possibly configurability of
the publish dialog if people decide they want it.
--
Petr "Pasky" Baudis
People who take cold baths never have rheumatism, but they have
cold baths.
^ permalink raw reply
* [PATCH] git-gui: Mark-up strings in show_{other,unmerged}_diff() for localization
From: Johannes Sixt @ 2008-10-03 11:13 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Git Mailing List
From: Johannes Sixt <johannes.sixt@telecom.at>
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
lib/diff.tcl | 22 +++++++++++-----------
1 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/lib/diff.tcl b/lib/diff.tcl
index abe502d..484ebb4 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -117,22 +117,22 @@ proc show_unmerged_diff {cont_info} {
if {$merge_stages(2) eq {}} {
set is_conflict_diff 1
lappend current_diff_queue \
- [list "LOCAL: deleted\nREMOTE:\n" d======= \
+ [list [mc "LOCAL: deleted\nREMOTE:\n"] d======= \
[list ":1:$current_diff_path" ":3:$current_diff_path"]]
} elseif {$merge_stages(3) eq {}} {
set is_conflict_diff 1
lappend current_diff_queue \
- [list "REMOTE: deleted\nLOCAL:\n" d======= \
+ [list [mc "REMOTE: deleted\nLOCAL:\n"] d======= \
[list ":1:$current_diff_path" ":2:$current_diff_path"]]
} elseif {[lindex $merge_stages(1) 0] eq {120000}
|| [lindex $merge_stages(2) 0] eq {120000}
|| [lindex $merge_stages(3) 0] eq {120000}} {
set is_conflict_diff 1
lappend current_diff_queue \
- [list "LOCAL:\n" d======= \
+ [list [mc "LOCAL:\n"] d======= \
[list ":1:$current_diff_path" ":2:$current_diff_path"]]
lappend current_diff_queue \
- [list "REMOTE:\n" d======= \
+ [list [mc "REMOTE:\n"] d======= \
[list ":1:$current_diff_path" ":3:$current_diff_path"]]
} else {
start_show_diff $cont_info
@@ -218,17 +218,17 @@ proc show_other_diff {path w m cont_info} {
d_@
} else {
if {$sz > $max_sz} {
- $ui_diff insert end \
-"* Untracked file is $sz bytes.
-* Showing only first $max_sz bytes.
-" d_@
+ $ui_diff insert end [mc \
+"* Untracked file is %d bytes.
+* Showing only first %d bytes.
+" $sz $max_sz] d_@
}
$ui_diff insert end $content
if {$sz > $max_sz} {
- $ui_diff insert end "
-* Untracked file clipped here by [appname].
+ $ui_diff insert end [mc "
+* Untracked file clipped here by %s.
* To see the entire file, use an external editor.
-" d_@
+" [appname]] d_@
}
}
$ui_diff conf -state disabled
--
1.6.0.2.319.gffa7c
^ permalink raw reply related
* Re: [PATCHv4] gitweb: parse project/action/hash_base:filename PATH_INFO
From: Jakub Narebski @ 2008-10-03 10:31 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Petr Baudis, Junio C Hamano, Shawn O. Pearce
In-Reply-To: <cb7bb73a0810022304r11d2ad87q7691213ff67f7e4c@mail.gmail.com>
On Fri, 3 Oct 2008, Giuseppe Bilotta wrote:
> On Fri, Oct 3, 2008 at 2:48 AM, Jakub Narebski <jnareb@gmail.com> wrote:
>> On Thu, 2 Oct 2008, Giuseppe Bilotta wrote:
>>> But implementing the path_info parsing first makes the input param
>>> refactoring SO much nicer that I would rather put a comment here
>>> saying "this code sucks: we should rather collect all input
>>> parameters" and then clean it up on the subsequent patch.
>>
>> Why not cleanup first?
>
> Because cleaning it up depends on the refactoring, and the refactoring
> is much cleaner when path_info already handles $action too.
Hmmm... it looks like after refactoring you don't have to change href()
at all when adding $action to parameters gitweb can get from path_info;
all that having refactoring (cleanup) upfront changes in/for this patch
is a bit different saving action (also in %input_params) and lack of
this ugly and I think subtly wrong added code for href(...,-replay=>1).
>> When implementing href(..., -replay=>1) I have forgot that some of
>> gitweb parameters are implicitly passed ($project, because it is needed
>> in most gitweb links), and some can be passed via path_info ($hash
>> and/or $hash_base, $file_name). Your code adds $action to the mix, but
>> it doesn't change the fact that 1.) even before your code -replay case
>> was incorrect for some path_info links (handcrafted, as gitweb generates
>> only $project via path_info); 2.) code you have added is a bit ugly.
>>
>> Besides using variables change a little meaning of -replay, namely
>> in your code gitweb always sets action, even for non-path_name links
>> when we started from "default action" (i.e. without action set) links.
>> I guess this is mainly theoretical issue, as I don't think that default
>> views use many -replay links.
>
> Ah the issue of the default action is something I hadn't taken into
> consideration, actually. Now the question is, should replay keep
> default -> default, or should it go with default -> last incantation?
I think we should use default -> default.
Besides I think there can also be an issue ("can" because I am not sure
if in practice it is a problem) the fact that gitweb sometimes expand
parameters to sha-1, for example setting $head to git_get_head_hash()
if it is not set (default param value), and not to 'HEAD'. This perhaps
should be changed, but to be on safer side better not to use 'action
variables' because some code treats them as temporary variables.
>> P.S. with the idea of pushing parameters obtained not from CGI query
>> string to $cgi->param() via "$cgi->param($name, $value);" or in named
>> params form "$cgi->(-name=>$name, -value=>$value);" you would not need
>> to change (a bit hacky, admittedly) href(...,-replay=>1) code.
>
> Yes, but it would muddy the waters about 'where did this parameter
> come from' in case we ever need to know that.
True. Like for example implementing -faithful_replay where if parameter
was passed through path_info it is replayed through path_info, and if
it was passed through query string it is replayed as CGI query param.
After thinking a bit about that I think that %input_params idea is
superior to both $cgi->params(-name=>..., -value=>...) and to have
either no strict refs $$name or name to action variable ref hash.
See also my comment for the refactoring patch.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCHv4] gitweb: refactor input parameters parse/validation
From: Jakub Narebski @ 2008-10-03 11:20 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Petr Baudis, Junio C Hamano, Shawn O. Pearce
In-Reply-To: <cb7bb73a0810030024t663940dbqa23f2e1bb75e23bc@mail.gmail.com>
On Fri, 3 Oct 2008, Giuseppe Bilotta wrote:
> On Fri, Oct 3, 2008 at 3:36 AM, Jakub Narebski <jnareb@gmail.com> wrote:
>> On Thu, 2 Oct 2008, Giuseppe Bilotta wrote:
>>> We thus collect all the parameters into the new %input_params hash,
>>> removing the need for ad-hoc kludgy code in href().
>>
>> Alternate solution would be push data from PATH_INFO in query params
>> data using (for example)
>>
>> $cgi->param('a', $action);
>>
>> or, naming parameters explicitely
>>
>> $cgi->param(-name=>'a', -value=>$action);
>>
>> This avoids need for additional variable, reuses current code, and
>> nicely sidesteps issue whether to use long names for keys ('action',
>> 'file_name') or short ones from CGI query ('a', 'f').
>>
>>
>> It would probably has to be at least partially written to check which
>> of those two solutions (%input_params or $cgi->param('a', $a)))
>> is better.
>
> If we really don't want to care where the parameters came from *at
> all*, writing on $cgi->param is likely to be the cleanest solution.
I now think that %input_params is a better solution, even if we won't
be implementing -true_replay / -faithful_replay option, where options
gotten in path_info would be passed in path_info, and options passed
in query string would be passed as CGI params (even if those params
could be encoded in path_info).
One of the advantages is that having @cgi_param_mapping near the top
provides opportunity to describe short CGI query options.
Another advantage is that with %input_params it is easy to find if
parameter is multivalued, or is meant to be multivalued: just check
ref($input_params{long_name}); well, perhaps not _that_ easy, but...
>>> As much of the
>>> parameters validation code is now shared between CGI and PATH_INFO,
>>> although this requires PATH_INFO parsing to be interleaved into the main
>>> code instead of being factored out into its own routine.
>>
>> I'm not sure if it is worth it then to unify parameters validation,
>> with such drawback.
>
> Yeah, I don't like it very much either. But it does spare a little on
> the validation. OTOH, the amount we spare is not extraordinary, so
> it's probably not worth the spaghettization of the code ...
Avoiding repetition can be done in different ways, for example we can
have gitweb assume that if value is already in params variable
($project, $action, $hash,...) it is already validated, and if it is
only in %input_params it is not. Add to that for example putting
common part of project path checks into validate_project and you have
all the advantages (no code repetition, spared validation) without
drawback of harder to maintain spaghetti-like code.
>>> +# fill %input_params with the CGI parameters. All values except for 'opt'
>>> +# should be single values, but opt can be an array. We should probably
>>> +# build an array of parameters that can be multi-valued, but since for the time
>>> +# being it's only this one, we just single it out
>>> +while (my ($name, $symbol) = each %cgi_param_mapping) {
>>> + if ($symbol eq 'opt') {
>>> + $input_params{$name} = [$cgi->param($symbol)];
>>> + } else {
>>> + $input_params{$name} = $cgi->param($symbol);
>>> }
>>> }
>>
>> If it was chosen to use short (CGI query) parameter names, the above
>> loop could be replaced by simple
>>
>> %input_params = $cgi->Vars;
>>
>> or to be more exact, if we want to have multi-valued parameters stored
>> as array ref
>>
>> %input_params = map { [ split /\0/ ] if /\0/; } $cgi->Vars;
>>
>>
>> See CGI(3pm):
>>
>> When using this [Vars], the thing you must watch out for are multivalued CGI
>> parameters. Because a hash cannot distinguish between scalar and list con-
>> text, multivalued parameters will be returned as a packed string, separated
>> by the "\0" (null) character. You must split this packed string in order
>> to get at the individual values.
>
> Ah, an interesting alternative. This would make parameter copying a
> one-liner, almost as good as just using $cgi->param for everything :)
Hmmmm...
[cut]
>> Note that this code does less checking if $project is in path_info than
>> for the case where it is set by CGI query. Perhaps there should be base
>> fast check in a loop, and more extensive test later.
>
> Uh ... isn't this exactly what's happening? In the loop we just gobble
> until we find a git dir. Validation is then done, and it's the _same_
> validation for both cases. Why do you say that path_info $project is
> less checked?
For project from query string we now have:
!validate_pathname($project) ||
!(-d "$projectroot/$project") ||
!check_head_link("$projectroot/$project") ||
($export_ok && !(-e "$projectroot/$project/$export_ok")) ||
($strict_export && !project_in_list($project))
For project from path_info we now have:
while ($project && !check_head_link("$projectroot/$project")) {
$project =~ s,/*[^/]*$,,;
}
...
!validate_pathname($project) ||
($export_ok && !-e "$projectroot/$project/$export_ok") ||
($strict_export && !project_in_list($project))
It is almost the same; I have thought that they differ more. Perhaps
some of above code could be refactored into validate_project(), or
project_ok() subroutine.
>>> $params{'project'} = $project unless exists $params{'project'};
>>>
>>> if ($params{-replay}) {
>>> - while (my ($name, $symbol) = each %mapping) {
>>> - if (!exists $params{$name}) {
>>> - # the parameter we want to recycle may be either part of the
>>> - # list of CGI parameter, or recovered from PATH_INFO
>>> - if ($cgi->param($symbol)) {
>>> - # to allow for multivalued params we use arrayref form
>>> - $params{$name} = [ $cgi->param($symbol) ];
>>> - } else {
>>> - no strict 'refs';
>>> - $params{$name} = $$name if $$name;
>>> - }
>>> - }
>>> + while (my ($name, $val) = each %input_params) {
>>> + $params{$name} = $input_params{$name}
>>> + unless (exists $params{$name});
>>
>> Very nice, short code. Should be something like that from
>> the very beginning.
>
> Ok, I'll try working up a patch for params merging before any
> path_info extensions.
Perhaps also put query string handling into evaluate_CGI_query(), or
evaluate_query_string(), similar to evaluate_path_info()?
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCHv4] gitweb: generate project/action/hash URLs
From: Jakub Narebski @ 2008-10-03 11:24 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git
In-Reply-To: <cb7bb73a0810022330l498bdb20h703dec7833a443e@mail.gmail.com>
On Fri, 3 October 2008, Giuseppe Bilotta wrote:
> On Fri, Oct 3, 2008 at 3:48 AM, Jakub Narebski <jnareb@gmail.com> wrote:
>> On Tue, 2 Oct 2008, Giuseppe Bilotta wrote:
>>> + # Finally, we put either hash_base:file_name or hash
>>> + if (defined $params{'hash_base'}) {
>>> + $href .= "/".esc_url($params{'hash_base'});
>>> + if (defined $params{'file_name'}) {
>>> + $href .= ":".esc_url($params{'file_name'});
>>> + delete $params{'file_name'};
>>> + }
>>> + delete $params{'hash'};
>>> + delete $params{'hash_base'};
>>> + } elsif (defined $params{'hash'}) {
>>> + $href .= "/".esc_url($params{'hash'});
>>> + delete $params{'hash'};
>>> + }
>>
>> Hmmmm...
>>
>> Shouldn't the code first check for $file_name, then add either
>> "$hash_base:$file_name" (url-escaped), or "$hash" (not "$hash_base")?
>
> Hm, your idea is probably better indeed, if we can ensure that
> $file_name is always set for generated links that need $hash_base (I
> mean, as opposed to the root tree case we were discussing for the
> first patch).
Hmm... I'm just worrying here about diluting meaning of params passed
via path_info. We had either project/hash, or project/hash_base:file_name;
now the hashy parameter in path_info can be hash or hash_base and we
don't know which. But perhaps I am worrying over nothing...
A short comment though would be nice (that we can have $hash or
$hash_base for case without $file_name)
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCHv4] gitweb: use_pathinfo filenames start with /
From: Jakub Narebski @ 2008-10-03 11:28 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Petr Baudis, Junio C Hamano, Shawn O. Pearce
In-Reply-To: <1222906234-8182-5-git-send-email-giuseppe.bilotta@gmail.com>
On Thu, 2 Oct 2008, Giuseppe Bilotta wrote:
> When using path info, make filenames start with a / (right after the :
> that separates them from the hash base). This minimal change allows
> relative navigation to work properly when viewing HTML files.
...in 'raw' mode/'blob_plain' view.
This allows to for example view web site as it was at some revision,
following relative links and checking if they are broken.
>
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Acked-by: Jakub Narebski <jnareb@gmail.com>
(Note that it depends on previous patch, and without it doesn't make
sense, so this Ack doesn't matter much now!)
> ---
> gitweb/gitweb.perl | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 2c380ac..3e5b2b7 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -690,7 +690,7 @@ sub href (%) {
> # try to put as many parameters as possible in PATH_INFO:
> # - project name
> # - action
> - # - hash or hash_base:filename
> + # - hash or hash_base:/filename
>
> # Strip any trailing / from $href, or we might get double
> # slashes when the script is the DirectoryIndex
> @@ -708,11 +708,11 @@ sub href (%) {
> delete $params{'action'};
> }
>
> - # Finally, we put either hash_base:file_name or hash
> + # Finally, we put either hash_base:/file_name or hash
> if (defined $params{'hash_base'}) {
> $href .= "/".esc_url($params{'hash_base'});
> if (defined $params{'file_name'}) {
> - $href .= ":".esc_url($params{'file_name'});
> + $href .= ":/".esc_url($params{'file_name'});
> delete $params{'file_name'};
> }
> delete $params{'hash'};
> --
> 1.5.6.5
>
>
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: How to remove a commit object?
From: Klas Lindberg @ 2008-10-03 11:42 UTC (permalink / raw)
To: Johannes Sixt
Cc: Jakub Narebski, Michael J Gruber, Steven Grimm, Git Users List
In-Reply-To: <48E4E27E.7030308@viscovery.net>
On Thu, Oct 2, 2008 at 5:02 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>> with filter-branch, I would appreciate some guidance for using it. It
>> basically seemed to do exactly what I wanted (recreate the repo, minus
>> some explicit stuff, with history intact otherwise), except the result
>> looked crazy.
>
> And your definition of 'crazy' is...?
Right... :-)
Crazy == Obviously incorrect behaviour that I didn't analyze. Out of
167 commits on subdirectory B, only 14 survived the filtering.
I tried "git filter-branch --tree-filter 'rm -rf <list of everything
except B>' HEAD" instead, but I can't use that. The change history for
all the non-B paths are still in the repo afterwards, and thus you can
easily recreate any file outside subdirectory B.
Is there some way to do what I need with git-filter-branch today, or
must I wait until 1.6.1 is released?
BR / Klas
> I assume that you used --subdirectory-filter. This has issues that will be
> fixed in 1.6.1. You need a current 'master' git (at least b805ef08).
>
> -- Hannes
>
^ permalink raw reply
* Re: How to remove a commit object?
From: Johannes Sixt @ 2008-10-03 12:03 UTC (permalink / raw)
To: Klas Lindberg
Cc: Jakub Narebski, Michael J Gruber, Steven Grimm, Git Users List
In-Reply-To: <33f4f4d70810030442l5042fdbfw18f97336c5a331cc@mail.gmail.com>
Klas Lindberg schrieb:
> On Thu, Oct 2, 2008 at 5:02 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>> I assume that you used --subdirectory-filter. This has issues that will be
>> fixed in 1.6.1. You need a current 'master' git (at least b805ef08).
>
> Is there some way to do what I need with git-filter-branch today, or
> must I wait until 1.6.1 is released?
You can remove all occurences of the "--full-history" flag from your
/usr/libexec/git-core/git-filter-branch script. This is sufficient for
some repositories because this triggers the bug less often. This means
that the resulting history may still be incorrect, but chances are higher
that it is correct.
Other than that, you can just clone git.git and compile it yourself. It's
a simple matter of "make prefix=$HOME/mytempgit install".
-- Hannes
^ permalink raw reply
* [PATCH] builtin-commit: avoid always using reduce_heads()
From: Miklos Vajna @ 2008-10-03 12:04 UTC (permalink / raw)
To: spearce; +Cc: SZEDER Gabor, jnareb, Johannes.Schindelin, git
In-Reply-To: <20081003023518.GA3291@spearce.org>
In case git merge --no-ff is used with --no-commit or we have a
conflict, write info about if fast forwards are allowed or not to
$GIT_DIR/MERGE_MODE.
Based on this info, don't run reduce_heads() in case fast-forwards are
denied, to avoid turning a 'merge --no-ff' to a non-merge commit.
Test case by SZEDER Gabor <szeder@ira.uka.de>
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
On Thu, Oct 02, 2008 at 07:35:18PM -0700, "Shawn O. Pearce" <spearce@spearce.org> wrote:
> > + unlink(git_path("MERGE_MODE"));
> > unlink(git_path("SQUASH_MSG"));
> >
> > if (commit_index_files())
>
> Hmmph. Should branch.c and builtin-reset.c clean this new file
> up too?
Right, I added it to branch.c::remove_branch_state() and
builtin-merge.c::drop_save(). I don't think I should touch
builtin-reset.c, it does not delete MERGE_HEAD/MERGE_MSG either.
> > + fd = open(git_path("MERGE_MODE"), O_WRONLY | O_CREAT,
> > 0666);
> > + if (fd < 0)
> > + die("Could open %s for writing",
> > git_path("MERGE_MODE"));
> > + strbuf_reset(&buf);
> > + if (!allow_fast_forward)
> > + strbuf_addf(&buf, "no-ff");
> > + if (write_in_full(fd, buf.buf, buf.len) != buf.len)
>
> Shouldn't we open this file with O_TRUNC to avoid this scenario:
>
> $ git merge --no-ff --no-commit foo
> $ git reset --hard
> $ git merge --no-commit foo
> ... *sigh* MERGE_MODE still has "no-ff" in it ...
>
> This is especially true since some porcelain (e.g. git-gui) just
> deletes MERGE_HEAD right now and doesn't know about cleaning up
> MERGE_MODE. We'd want to at least reset it correctly on the next
> invocation to git-merge.
Fixed.
branch.c | 1 +
builtin-commit.c | 13 ++++++++++++-
builtin-merge.c | 10 ++++++++++
t/t7600-merge.sh | 9 +++++++++
4 files changed, 32 insertions(+), 1 deletions(-)
diff --git a/branch.c b/branch.c
index b1e59f2..205b89d 100644
--- a/branch.c
+++ b/branch.c
@@ -168,5 +168,6 @@ void remove_branch_state(void)
unlink(git_path("MERGE_HEAD"));
unlink(git_path("MERGE_RR"));
unlink(git_path("MERGE_MSG"));
+ unlink(git_path("MERGE_MODE"));
unlink(git_path("SQUASH_MSG"));
}
diff --git a/builtin-commit.c b/builtin-commit.c
index 55e1087..f546cf7 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -937,6 +937,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
unsigned char commit_sha1[20];
struct ref_lock *ref_lock;
struct commit_list *parents = NULL, **pptr = &parents;
+ struct stat statbuf;
+ int allow_fast_forward = 1;
git_config(git_commit_config, NULL);
@@ -988,7 +990,15 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
reflog_msg = "commit";
pptr = &commit_list_insert(lookup_commit(head_sha1), pptr)->next;
}
- parents = reduce_heads(parents);
+ strbuf_reset(&sb);
+ if (!stat(git_path("MERGE_MODE"), &statbuf)) {
+ if (strbuf_read_file(&sb, git_path("MERGE_MODE"), 0) < 0)
+ die("could not read MERGE_MODE: %s", strerror(errno));
+ if (!strcmp(sb.buf, "no-ff"))
+ allow_fast_forward = 0;
+ }
+ if (allow_fast_forward)
+ parents = reduce_heads(parents);
/* Finally, get the commit message */
strbuf_init(&sb, 0);
@@ -1040,6 +1050,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
unlink(git_path("MERGE_HEAD"));
unlink(git_path("MERGE_MSG"));
+ unlink(git_path("MERGE_MODE"));
unlink(git_path("SQUASH_MSG"));
if (commit_index_files())
diff --git a/builtin-merge.c b/builtin-merge.c
index 5c65a58..4c9ed5d 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -180,6 +180,7 @@ static void drop_save(void)
{
unlink(git_path("MERGE_HEAD"));
unlink(git_path("MERGE_MSG"));
+ unlink(git_path("MERGE_MODE"));
}
static void save_state(void)
@@ -1210,6 +1211,15 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
merge_msg.len)
die("Could not write to %s", git_path("MERGE_MSG"));
close(fd);
+ fd = open(git_path("MERGE_MODE"), O_WRONLY | O_CREAT | O_TRUNC, 0666);
+ if (fd < 0)
+ die("Could open %s for writing", git_path("MERGE_MODE"));
+ strbuf_reset(&buf);
+ if (!allow_fast_forward)
+ strbuf_addf(&buf, "no-ff");
+ if (write_in_full(fd, buf.buf, buf.len) != buf.len)
+ die("Could not write to %s", git_path("MERGE_MODE"));
+ close(fd);
}
if (merge_was_ok) {
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 9516f54..98cfc53 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -511,4 +511,13 @@ test_expect_success 'in-index merge' '
test_debug 'gitk --all'
+test_expect_success 'merge --no-ff --no-commit && commit' '
+ git reset --hard c0 &&
+ git merge --no-ff --no-commit c1 &&
+ EDITOR=: git commit &&
+ verify_parents $c0 $c1
+'
+
+test_debug 'gitk --all'
+
test_done
--
1.6.0.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox