Git development
 help / color / mirror / Atom feed
* Re: [PATCH V4] git on Mac OS and precomposed unicode
From: Torsten Bögershausen @ 2012-01-29 10:29 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Junio C Hamano, git, Torsten Bögershausen
In-Reply-To: <CACsJy8BKQHLdoXfSKsULkWWbWjWEuZgr=bVNKmgCSArvwbf2UA@mail.gmail.com>

On 22.01.12 10:58, Nguyen Thai Ngoc Duy wrote:
> On Sun, Jan 22, 2012 at 5:56 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> [Pinging Nguyen who has worked rather extensively on the start-up sequence
>> for ideas.]
>>
[snip]
> 
> I just have a quick look, you reencode opendir, readdir, and
> closedir() to precomposed form. But files are still in decomposed
> form, does open(<precomposed file>) work when only <decomposed file>
> exists?

Yes. All function like stat(), lstat(), open(), fopen(), unlink() behave the same
for precomped or decomposed. This is similar to the ignore case feature.
And because the default HFS+ is case preserving, case insenstive and unicode decomposing
all at the same time, a file name "Ä" could be reached under 4 different names.
Please see the output of the test script:
(which is at the end of this email)

tests/Darwin_i386/NFC file name created as nfc is readable as nfd
tests/Darwin_i386/NFC readdir returns nfd but expected is nfc
tests/Darwin_i386/NFD file name created as nfd is readable as nfc
tests/Darwin_i386/NFCNFD 1 file found in directory, but there should be 2
tests/Darwin_i386/NFCNFD nfc is missing, nfd is present
tests/Darwin_i386/NFCNFD nfc File content overwritten by nfd
tests/Darwin_i386/NFDNFC 1 file found in directory, but there should be 2
tests/Darwin_i386/NFDNFC nfc is missing, nfd is present
tests/Darwin_i386/NFDNFC nfd File content overwritten by nfc


> 
>>> In order to prevent that ever a file name in decomposed unicode is
>>> entering the index, a "brute force" attempt is taken: all arguments into
>>> git (argv[1]..argv[n]) are converted into precomposed unicode.  This is
>>> done in git.c by calling precompose_argv().  This function is actually a
>>> #define, and it is only defined under Mac OS.  Nothing is converted on
>>> any other platforms.
> 
> This is not entirely safe. Filenames can be taken from a file for
> example (--stdin option or similar). Unless I'm mistaken, all file
> names must enter git through the index, the conversion at read-cache.c
> may be a better option.
Good point, thanks. 
I added some code to read-cache.c, and it works for files, but not for directories.
I looked through the code for "case-ignoring" directory names, and couldn't
find something obvious. More work is to be done.
 

[snip]
> I'd rather encode at index level and read_directory() than at argv[].
>But if reencoding argv is the only feasible way, perhaps put the
>conversion in parse_options()?

I tried that, and found that git-lsfiles.c doesn't use parse_options.

[snip]

On the long run I want to get rid of the argv[] conversion completely,
but I'm not there yet.

Thanks for all comments and inspiration!

(and apologies for my long response times I use to have)
/Torsten



PS:
Here the script.
Mac OS writes decomposd unicode to HFS+, precomposed unicode to VFAT and SAMBA.
In any case readdir() returns decomposed.

=================
#!/bin/sh
errorandout() {
  echo Error: The shell can not handle nfd
  echo try to run /bin/bash $0
  rm -rf $DIR
  exit 1
}

checkDirNfcOrNfd() {
  DDNFCNFD=$1
  readdirexp=$2
  if test -r $DDNFCNFD/$aumlnfc; then
    x=`cat $DDNFCNFD/$aumlnfc`
    if test "$x" = nfd; then
      echo $DDNFCNFD file name created as nfd is readable as nfc
    fi
  fi
  if test -r $DDNFCNFD/$aumlnfd; then
    x=`cat $DDNFCNFD/$aumlnfd 2>/dev/null` || {
      echo $DDNFCNFD nfd is not readable, but readdir says that it exist
    }
    if test "$x" = nfc; then
      echo $DDNFCNFD file name created as nfc is readable as nfd
    fi
  fi
  readdirres=`echo $DDNFCNFD/*`
  if test "$readdirres" != "$DDNFCNFD/$readdirexp"; then
    if test "$readdirres" = $DDNFCNFD/$aumlnfd; then
      echo $DDNFCNFD readdir returns nfd but expected is nfc
    fi
    if test "$readdirres" = $DDNFCNFD/$aumlnfc; then
      echo $DDNFCNFD readdir returns nfc but expected is nfd
    fi
  fi
}

checkdirnfcnfd() {
  DDNFCNFD=$1
  if test `ls -1 $DDNFCNFD | wc -l` != 2; then
    if test `ls -1 $DDNFCNFD | wc -l` == 1; then
      echo $DDNFCNFD 1 file found in directory, but there should be 2
    else
      echo $DDNFCNFD 2 files should be in directory
    fi  
  fi

  x=`echo $DDNFCNFD/*`
  a=`echo $DDNFCNFD/$aumlnfd $DDNFCNFD/$aumlnfc`
  b=`echo $DDNFCNFD/$aumlnfc $DDNFCNFD/$aumlnfd`
  c=`echo $DDNFCNFD/$aumlnfc $DDNFCNFD/$aumlnfc`
  d=`echo $DDNFCNFD/$aumlnfd $DDNFCNFD/$aumlnfd`
  e=`echo $DDNFCNFD/$aumlnfc`
  f=`echo $DDNFCNFD/$aumlnfd`
  case "$x" in
    $a)
    ;;      
    $b)
    ;;
    $c)
    echo $DDNFCNFD nfd is hidden, nfc is listed twice
    ;;
    $d)
    echo $DDNFCNFD nfc is hidden, nfd is listed twice
    ;;
    $e)
    echo $DDNFCNFD nfd is missing, nfc is present
    ;;      
    $f)
    echo $DDNFCNFD nfc is missing, nfd is present
    ;;      
    *)
    echo $DDNFCNFD x`echo $x | xxd`
    ;;
  esac

  if ! test -r $DDNFCNFD/$aumlnfc; then
    echo $DDNFCNFD/nfc File does not exist
  else
    x=`cat $DDNFCNFD/$aumlnfc`
    if test "$x" != nfc; then
      echo $DDNFCNFD nfc File content overwritten by $x
    fi
  fi
  
  if ! test -r $DDNFCNFD/$aumlnfd; then
    echo $DDNFCNFD/nfd File does not exist
  else
    x=`cat $DDNFCNFD/$aumlnfd`
    if test "$x" != nfd; then
      echo $DDNFCNFD nfd File content overwritten by $x
    fi
  fi
}


aumlnfc=$(printf '\303\204')
aumlnfd=$(printf '\101\314\210')

DIR=tests/`uname -s`_`uname -m`
echo "DIR=$DIR"

rm -rf $DIR/NFC &&
rm -rf $DIR/NFD &&
rm -rf $DIR/NFCNFD &&
rm -rf $DIR/NFDNFC &&
mkdir -p $DIR/NFC &&
mkdir -p $DIR/NFD &&
mkdir -p $DIR/NFDNFC &&
mkdir -p $DIR/NFCNFD &&
echo nfc > $DIR/NFC/$aumlnfc &&
echo nfd > $DIR/NFD/$aumlnfd &&
echo nfd > $DIR/NFDNFC/$aumlnfd &&
echo nfc > $DIR/NFDNFC/$aumlnfc &&
echo nfc > $DIR/NFCNFD/$aumlnfc &&
echo nfd > $DIR/NFCNFD/$aumlnfd && {
    # test 1: basic if the shell handles nfd
    if ! test -r $DIR/NFD/$aumlnfd; then
      errorandout
    fi

  for DD in tests/*; do
    checkDirNfcOrNfd $DD/NFC  $aumlnfc
    checkDirNfcOrNfd $DD/NFD  $aumlnfd

    checkdirnfcnfd $DD/NFCNFD
    checkdirnfcnfd $DD/NFDNFC
  done
} || errorandout

^ permalink raw reply

* Re: [PATCH v3] gitweb: add project_filter to limit project list to a subdirectory
From: Jakub Narebski @ 2012-01-29 12:54 UTC (permalink / raw)
  To: Bernhard R. Link; +Cc: git
In-Reply-To: <20120129012234.GD16079@server.brlink.eu>

On Sun, 29 Jan 2012, Bernhard R. Link wrote:

> This commit changes the project listing views (project_list,
> project_index and opml) to limit the output to only projects in a
> subdirectory if the new optional parameter ?pf=directory name is used.
> 
> The change is quite minimal as git_get_projects_list already can limit
> itself to a subdirectory (though that was previously only used for
> 'forks').
>
Nice and succinct.
 
> If there is a GITWEB_LIST file, the contents are just filtered like
> with the forks action.
> 
O.K.

> Without a GITWEB_LIST file only the given subdirectory is searched
> for projects (like with forks) unless GITWEB_STRICT_EXPORT is enabled.
> In the later case GITWEB_PROJECTROOT is traversed normally (unlike
> with forks) and projects not in the directory ignored.
> (As there is no check if the filter_path would have been found in
> the usual search as the project path is checked with forks).
> 
Now I understand how project_filter interacts with strict_export.

Though I am not sure if this "paranoid mode" is really necessary.  I don't
see how you could get in situation where scanning from $project_list and
filtering with $project_filter prefix, and scanning from 
$project_list/$project_filter would give different results.

I think you are overly paranoid here, but perhaps it is better to be
overly strict, and then relax it if it turns out to be not necessary.

> Reusing $project instead of adding a new parameter would have been
> nicer from a UI point-of-view (including PATH_INFO support) but
> would complicate the $project validating code that is currently being
> used to ensure nothing is exported that should not be viewable.
> 
Sidenote: support for actionless PATH_INFO URLs would make it even more
complicated...

> Additionally change html page headers to not only link the project
> root and the currently selected project but also the directories in
> between using project_filter.
> 
Excuse me changing my mind, but I think that as far as this patch series
is applied as whole, it would be better for maintability to keep those
two patches split; though put the above as a [part of] commit message
in 2/2 patch.

> Signed-off-by: Bernhard R. Link <brlink@debian.org>
> ---
> 
> changes since v2:
>         improve description
>         remove || 0 for boolean argument
>         merge with patch using this feature
>         use user-visible configuration names instead of internal ones
> 
> * Jakub Narebski <jnareb@gmail.com> [120128 23:45]:
> > "Bernhard R. Link" <brl+git@mail.brlink.eu> writes:
> > > If strict_export is enabled and there is no projects_list, it still
> > > traverses the full tree and only filters afterwards to avoid anything
> > > getting visible by this. Otherwise only the subtree needs to be
> > > traversed, significantly reducing load times.
> > >
> > I still don't understand interaction between project_filter ('pf'),
> > $strict_export and $projects_list being either directory or a file
> > with a list of projects.
> > 
> > Does it mean, that when $projects_list is a file with a list of projects,
> > and we use project_filter, then:
> > 
> > * if $strict_export is false, then $project_list is ignored, and the
> >   filtered list of projects is created by scanning
> >   "$projectroot/$project_filter"
> 
> No. If project_list is set, i.e. a file, then this is always used.
> If it is a directory (because it is not set thus set to projectroot),
> then with forks it still traverses that directory (as that was checked
> before to be a reachable project with a previous call to
> git_get_projects_list). In the case of project_filter only the directory
> is traversed without strict_export and the whole projectroot is
> traversed with strict_export.
> 
O.K., now I understand it.

> Is the new description better.
> 
Yes it is.

> > A few nitpicks with respect to patch itself.
> > 
> > >  -2827,6 +2835,7 @@ sub git_get_project_url_list {
> > >  
> > >  sub git_get_projects_list {
> > >  	my $filter = shift || '';
> > > +	my $paranoid = shift || 0;
> > >  	my @list;
> > >  
> > 
> > First, undefined value is false in Perl, so there is no need for
> > " || 0" in setting $paranoid variable.
> 
> I thought it make it clearer that the argument might not be set and
> what the default is. But that is personal taste.

First, optional parameter defaults to false in the 'my $foo = shift;'
or equivalent form is (I think) idiomatic Perl.  Second, this way of
writing it is used through gitweb code (CodingGuidelines: imitate existing
coding practices). 
 
> > Second, why not use global variable $strict_export instead of adding
> > another parameter to git_get_projects_list()?
> 
> That would change the action=forks behaviour to traverse the whole
> projectroot two times. This way paranoia is only activated if
> strict_mode is set _and_ the argument was not yet checked to be
> reachable.

Thanks for explanation.
 
>  gitweb/gitweb.perl |   52 ++++++++++++++++++++++++++++++++++++++++++++--------
>  1 files changed, 44 insertions(+), 8 deletions(-)

Not that large for a new feature...

-- 
Jakub Narebski
Poland

^ permalink raw reply

* [PATCH] completion: --edit-description option for git-branch
From: Ralf Thielow @ 2012-01-29 12:55 UTC (permalink / raw)
  To: spearce; +Cc: git, Ralf Thielow

Signed-off-by: Ralf Thielow <ralf.thielow@googlemail.com>
---
 contrib/completion/git-completion.bash |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 1496c6d..e44eefd 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1152,7 +1152,7 @@ _git_branch ()
 		__gitcomp "
 			--color --no-color --verbose --abbrev= --no-abbrev
 			--track --no-track --contains --merged --no-merged
-			--set-upstream
+			--set-upstream --edit-description
 			"
 		;;
 	*)
-- 
1.7.9.dirty

^ permalink raw reply related

* Re: [PATCH V4] git on Mac OS and precomposed unicode
From: Torsten Bögershausen @ 2012-01-29 12:57 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: Nguyen Thai Ngoc Duy, Junio C Hamano, git
In-Reply-To: <4F251FA1.80400@web.de>


> I tried that, and found that git-lsfiles.c doesn't use parse_options.
Oops, I shouldn't have written that: git-lsfiles uses parse_options.

Sorry for the noise.
/torsten

^ permalink raw reply

* [PATCH v4 1/2] gitweb: add project_filter to limit project list to a subdirectory
From: Bernhard R. Link @ 2012-01-29 16:06 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <201201291354.50241.jnareb@gmail.com>

This commit changes the project listing views (project_list,
project_index and opml) to limit the output to only projects in a
subdirectory if the new optional parameter ?pf=directory name is used.

The change is quite minimal as git_get_projects_list already can limit
itself to a subdirectory (though that was previously only used for
'forks').

If there is a GITWEB_LIST file, the contents are just filtered like
with the forks action.

Without a GITWEB_LIST file only the given subdirectory is searched
for projects (like with forks) unless GITWEB_STRICT_EXPORT is enabled.
In the later case GITWEB_PROJECTROOT is traversed normally (unlike
with forks) and projects not in the directory ignored.
(As there is no check if the filter_path would have been found in
the usual search as the project path is checked with forks).

Reusing $project instead of adding a new parameter would have been
nicer from a UI point-of-view (including PATH_INFO support) but
would complicate the $project validating code that is currently being
used to ensure nothing is exported that should not be viewable.

Signed-off-by: Bernhard R. Link <brlink@debian.org>
---

* Jakub Narebski <jnareb@gmail.com> [120129 13:54]:
> On Sun, 29 Jan 2012, Bernhard R. Link wrote:
> Though I am not sure if this "paranoid mode" is really necessary.  I don't
> see how you could get in situation where scanning from $project_list and
> filtering with $project_filter prefix, and scanning from
> $project_list/$project_filter would give different results.
>
> I think you are overly paranoid here, but perhaps it is better to be
> overly strict, and then relax it if it turns out to be not necessary.

As far as I do understand it, this is the only (hopefully unecessary)
effect strict_export without a project_list has in gitweb, so I did not
want to remove that with this change.

> Excuse me changing my mind, but I think that as far as this patch series
> is applied as whole, it would be better for maintability to keep those
> two patches split; though put the above as a [part of] commit message
> in 2/2 patch.

Split again, though this time only the change for existing pages in the
second commit and the code duplication you spoke against removed.

 gitweb/gitweb.perl |   43 ++++++++++++++++++++++++++++++++++++-------
 1 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index abb5a79..f0e03d8 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -760,6 +760,7 @@ our @cgi_param_mapping = (
 	search_use_regexp => "sr",
 	ctag => "by_tag",
 	diff_style => "ds",
+	project_filter => "pf",
 	# this must be last entry (for manipulation from JavaScript)
 	javascript => "js"
 );
@@ -976,7 +977,7 @@ sub evaluate_path_info {
 
 our ($action, $project, $file_name, $file_parent, $hash, $hash_parent, $hash_base,
      $hash_parent_base, @extra_options, $page, $searchtype, $search_use_regexp,
-     $searchtext, $search_regexp);
+     $searchtext, $search_regexp, $project_filter);
 sub evaluate_and_validate_params {
 	our $action = $input_params{'action'};
 	if (defined $action) {
@@ -994,6 +995,13 @@ sub evaluate_and_validate_params {
 		}
 	}
 
+	our $project_filter = $input_params{'project_filter'};
+	if (defined $project_filter) {
+		if (!validate_pathname($project_filter)) {
+			die_error(404, "Invalid project_filter parameter");
+		}
+	}
+
 	our $file_name = $input_params{'file_name'};
 	if (defined $file_name) {
 		if (!validate_pathname($file_name)) {
@@ -2827,6 +2835,7 @@ sub git_get_project_url_list {
 
 sub git_get_projects_list {
 	my $filter = shift || '';
+	my $paranoid = shift;
 	my @list;
 
 	$filter =~ s/\.git$//;
@@ -2839,7 +2848,7 @@ sub git_get_projects_list {
 		my $pfxlen = length("$dir");
 		my $pfxdepth = ($dir =~ tr!/!!);
 		# when filtering, search only given subdirectory
-		if ($filter) {
+		if ($filter and not $paranoid) {
 			$dir .= "/$filter";
 			$dir =~ s!/+$!!;
 		}
@@ -2864,6 +2873,10 @@ sub git_get_projects_list {
 				}
 
 				my $path = substr($File::Find::name, $pfxlen + 1);
+				# paranoidly only filter here
+				if ($paranoid && $filter && $path !~ m!^\Q$filter\E/!) {
+					next;
+				}
 				# we check related file in $projectroot
 				if (check_export_ok("$projectroot/$path")) {
 					push @list, { path => $path };
@@ -3823,6 +3836,18 @@ sub print_header_links {
 	}
 }
 
+sub print_nav_breadcrumbs_path {
+	my $dirprefix = undef;
+	while (my $part = shift) {
+		$dirprefix .= "/" if defined $dirprefix;
+		$dirprefix .= $part;
+		print $cgi->a({-href => href(project => undef,
+		                             project_filter => $dirprefix,
+					     action=>"project_list")},
+			      esc_html($part)) . " / ";
+	}
+}
+
 sub print_nav_breadcrumbs {
 	my %opts = @_;
 
@@ -3841,6 +3866,8 @@ sub print_nav_breadcrumbs {
 			print " / $opts{-action_extra}";
 		}
 		print "\n";
+	} elsif (defined $project_filter) {
+		print_nav_breadcrumbs_path(split '/', $project_filter);
 	}
 }
 
@@ -3963,9 +3990,11 @@ sub git_footer_html {
 		}
 
 	} else {
-		print $cgi->a({-href => href(project=>undef, action=>"opml"),
+		print $cgi->a({-href => href(project=>undef, action=>"opml",
+		                             project_filter => $project_filter),
 		              -class => $feed_class}, "OPML") . " ";
-		print $cgi->a({-href => href(project=>undef, action=>"project_index"),
+		print $cgi->a({-href => href(project=>undef, action=>"project_index",
+		                             project_filter => $project_filter),
 		              -class => $feed_class}, "TXT") . "\n";
 	}
 	print "</div>\n"; # class="page_footer"
@@ -5979,7 +6008,7 @@ sub git_project_list {
 		die_error(400, "Unknown order parameter");
 	}
 
-	my @list = git_get_projects_list();
+	my @list = git_get_projects_list($project_filter, $strict_export);
 	if (!@list) {
 		die_error(404, "No projects found");
 	}
@@ -6018,7 +6047,7 @@ sub git_forks {
 }
 
 sub git_project_index {
-	my @projects = git_get_projects_list();
+	my @projects = git_get_projects_list($project_filter, $strict_export);
 	if (!@projects) {
 		die_error(404, "No projects found");
 	}
@@ -7855,7 +7884,7 @@ sub git_atom {
 }
 
 sub git_opml {
-	my @list = git_get_projects_list();
+	my @list = git_get_projects_list($project_filter, $strict_export);
 	if (!@list) {
 		die_error(404, "No projects found");
 	}
-- 
1.7.8.3

^ permalink raw reply related

* [PATCH 2/2] gitweb: place links to parent directories in page header
From: Bernhard R. Link @ 2012-01-29 16:07 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git

Change html page headers to not only link the project root and the
currently selected project but also the directories in between using
project_filter.

Signed-off-by: Bernhard R. Link <brlink@debian.org>
---
 gitweb/gitweb.perl |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index f0e03d8..e2a9146 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3853,7 +3853,10 @@ sub print_nav_breadcrumbs {
 
 	print $cgi->a({-href => esc_url($home_link)}, $home_link_str) . " / ";
 	if (defined $project) {
-		print $cgi->a({-href => href(action=>"summary")}, esc_html($project));
+		my @dirname = split '/', $project;
+		my $projectbasename = pop @dirname;
+		print_nav_breadcrumbs_path(@dirname);
+		print $cgi->a({-href => href(action=>"summary")}, esc_html($projectbasename));
 		if (defined $action) {
 			my $action_print = $action ;
 			if (defined $opts{-action_extra}) {
-- 
1.7.8.3

^ permalink raw reply related

* [PATCH v4 2/2] gitweb: place links to parent directories in page header
From: Bernhard R. Link @ 2012-01-29 16:13 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <20120129160615.GA13937@server.brlink.eu>

Change html page headers to not only link the project root and the
currently selected project but also the directories in between using
project_filter.

Signed-off-by: Bernhard R. Link <brlink@debian.org>
---
 gitweb/gitweb.perl |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index f0e03d8..e2a9146 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3853,7 +3853,10 @@ sub print_nav_breadcrumbs {
 
 	print $cgi->a({-href => esc_url($home_link)}, $home_link_str) . " / ";
 	if (defined $project) {
-		print $cgi->a({-href => href(action=>"summary")}, esc_html($project));
+		my @dirname = split '/', $project;
+		my $projectbasename = pop @dirname;
+		print_nav_breadcrumbs_path(@dirname);
+		print $cgi->a({-href => href(action=>"summary")}, esc_html($projectbasename));
 		if (defined $action) {
 			my $action_print = $action ;
 			if (defined $opts{-action_extra}) {
-- 
1.7.8.3

^ permalink raw reply related

* Re: [PATCH V4] git on Mac OS and precomposed unicode
From: Erik Faye-Lund @ 2012-01-29 16:26 UTC (permalink / raw)
  To: Carlos Martín Nieto; +Cc: Torsten Bögershausen, git
In-Reply-To: <1327184934.31804.32.camel@centaur.lab.cmartin.tk>

On Sat, Jan 21, 2012 at 11:28 PM, Carlos Martín Nieto <cmn@elego.de> wrote:
> On Sat, 2012-01-21 at 20:36 +0100, Torsten Bögershausen wrote:
>> * (Not all Windows versions support UTF-8 yet:
>>    Msysgit needs the unicode branch, cygwin supports UTF-8 since 1.7)
>
> This might be overly pedantic, but Windows doesn't really deal with
> UTF-8. To use Unicode you need to use the "wide" variant of the
> functions, and those take UTF-16.

This is exactly what the 'unicode'-branch in msysGit does, so the
comment is not incorrect at all.

^ permalink raw reply

* Re: [PATCH v4 1/2] gitweb: add project_filter to limit project list to a subdirectory
From: Jakub Narebski @ 2012-01-29 16:41 UTC (permalink / raw)
  To: Bernhard R. Link; +Cc: git
In-Reply-To: <20120129160615.GA13937@server.brlink.eu>

On Sun, 29 Jan 2012, Bernhard R. Link wrote:

> This commit changes the project listing views (project_list,
> project_index and opml) to limit the output to only projects in a
> subdirectory if the new optional parameter ?pf=directory name is used.
> 
> The change is quite minimal as git_get_projects_list already can limit
> itself to a subdirectory (though that was previously only used for
> 'forks').
> 
> If there is a GITWEB_LIST file, the contents are just filtered like
> with the forks action.
> 
> Without a GITWEB_LIST file only the given subdirectory is searched
> for projects (like with forks) unless GITWEB_STRICT_EXPORT is enabled.
> In the later case GITWEB_PROJECTROOT is traversed normally (unlike
> with forks) and projects not in the directory ignored.
> (As there is no check if the filter_path would have been found in
> the usual search as the project path is checked with forks).

I am still unsure if it is really necessary, but nevermind...
 
> Reusing $project instead of adding a new parameter would have been
> nicer from a UI point-of-view (including PATH_INFO support) but
> would complicate the $project validating code that is currently being
> used to ensure nothing is exported that should not be viewable.
> 
> Signed-off-by: Bernhard R. Link <brlink@debian.org>

Acked-by: Jakub Narebski <jnareb@gmail.com>

> ---
[...]
> @@ -3823,6 +3836,18 @@ sub print_header_links {
>  	}
>  }
>  
> +sub print_nav_breadcrumbs_path {
> +	my $dirprefix = undef;
> +	while (my $part = shift) {
> +		$dirprefix .= "/" if defined $dirprefix;
> +		$dirprefix .= $part;
> +		print $cgi->a({-href => href(project => undef,
> +		                             project_filter => $dirprefix,
> +					     action=>"project_list")},
> +			      esc_html($part)) . " / ";
> +	}
> +}
> +
>  sub print_nav_breadcrumbs {
>  	my %opts = @_;
>  
> @@ -3841,6 +3866,8 @@ sub print_nav_breadcrumbs {
>  			print " / $opts{-action_extra}";
>  		}
>  		print "\n";
> +	} elsif (defined $project_filter) {
> +		print_nav_breadcrumbs_path(split '/', $project_filter);
>  	}
>  }
>  

This could have been split into a separate 2/3 commit, but nevermind;
it can be squashed here.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH v4 2/2] gitweb: place links to parent directories in page header
From: Jakub Narebski @ 2012-01-29 16:46 UTC (permalink / raw)
  To: Bernhard R. Link; +Cc: git
In-Reply-To: <20120129161316.GD13937@server.brlink.eu>

On Sun, 29 Jan 2012, Bernhard R. Link wrote:

> Change html page headers to not only link the project root and the
> currently selected project but also the directories in between using
> project_filter.

Nice interface to the new feature... though it doesn't really address
the problem that gitweb homepage is slow to generate with large number
of projects.  Still, it is IMVHO a good improvement.
 
> Signed-off-by: Bernhard R. Link <brlink@debian.org>

Acked-by: Jakub Narebski <jnareb@gmail.com>

> ---
>  gitweb/gitweb.perl |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index f0e03d8..e2a9146 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -3853,7 +3853,10 @@ sub print_nav_breadcrumbs {
>  
>  	print $cgi->a({-href => esc_url($home_link)}, $home_link_str) . " / ";
>  	if (defined $project) {
> -		print $cgi->a({-href => href(action=>"summary")}, esc_html($project));
> +		my @dirname = split '/', $project;
> +		my $projectbasename = pop @dirname;
> +		print_nav_breadcrumbs_path(@dirname);
> +		print $cgi->a({-href => href(action=>"summary")}, esc_html($projectbasename));
>  		if (defined $action) {
>  			my $action_print = $action ;
>  			if (defined $opts{-action_extra}) {
> -- 

Nicely short with refactoring of print_nav_breadcrumbs_path() in 1/2!

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH v4 1/2] gitweb: add project_filter to limit project list to a subdirectory
From: Junio C Hamano @ 2012-01-29 21:06 UTC (permalink / raw)
  To: Bernhard R. Link; +Cc: Jakub Narebski, git
In-Reply-To: <20120129160615.GA13937@server.brlink.eu>

"Bernhard R. Link" <brl+git@mail.brlink.eu> writes:

> This commit changes the project listing views (project_list,
> project_index and opml) to limit the output to only projects in a
> subdirectory if the new optional parameter ?pf=directory name is used.
>
> The change is quite minimal as git_get_projects_list already can limit
> itself to a subdirectory (though that was previously only used for
> 'forks').
>
> If there is a GITWEB_LIST file, the contents are just filtered like
> with the forks action.

Meaning, a directory is shown if it is listed on GITWEB_LIST and is a
subdirectory of the directory specified with project_filter?  If so,
spelling it out instead of saying "just filtered like with the forks
action" may be clearer without making the description excessively longer.

> Without a GITWEB_LIST file only the given subdirectory is searched
> for projects (like with forks) unless GITWEB_STRICT_EXPORT is enabled.
> In the later case GITWEB_PROJECTROOT is traversed normally (unlike
> with forks) and projects not in the directory ignored.

It is unclear to me what "In the later case" refers to, even assuming that
it is a typo of "the latter case".

Do you mean "When there is no GITWEB_LIST but GITWEB_STRICT_EXPORT is set,
project_filter that specifies anything outside GITWEB_PROJECTROOT is
ignored"?

A more fundamental issue I have with this patch is how an end user starts
using this. Once project_filter is set, the breadcrumbs would let the user
click and navigate around, but in my superficial glance at the patch it is
not apparent how the initial setting of project_filter can happen without
the user manually adding pf= to the URL, which is a less than ideal end
user experience.

> @@ -2839,7 +2848,7 @@ sub git_get_projects_list {
>  		my $pfxlen = length("$dir");
>  		my $pfxdepth = ($dir =~ tr!/!!);
>  		# when filtering, search only given subdirectory
> -		if ($filter) {
> +		if ($filter and not $paranoid) {
>  			$dir .= "/$filter";
>  			$dir =~ s!/+$!!;
>  		}
> @@ -2864,6 +2873,10 @@ sub git_get_projects_list {
>  				}
>  
>  				my $path = substr($File::Find::name, $pfxlen + 1);
> +				# paranoidly only filter here
> +				if ($paranoid && $filter && $path !~ m!^\Q$filter\E/!) {
> +					next;
> +				}

When you find "foo" directory and a project_filter tells you to match
"foo", because $path does not match "^foo/", it will not match (even
though its subdirectory "foo/bar" would)?

> +sub print_nav_breadcrumbs_path {
> +	my $dirprefix = undef;
> +	while (my $part = shift) {
> +		$dirprefix .= "/" if defined $dirprefix;
> +		$dirprefix .= $part;
> +		print $cgi->a({-href => href(project => undef,
> +		                             project_filter => $dirprefix,
> +					     action=>"project_list")},
> +			      esc_html($part)) . " / ";
> +	}
> +}
> +
>  sub print_nav_breadcrumbs {
>  	my %opts = @_;
>  
> @@ -3841,6 +3866,8 @@ sub print_nav_breadcrumbs {
>  			print " / $opts{-action_extra}";
>  		}
>  		print "\n";
> +	} elsif (defined $project_filter) {
> +		print_nav_breadcrumbs_path(split '/', $project_filter);
>  	}
>  }

Hmm.

While this may not be wrong, I wonder if this is limiting a useful feature
too narrowly. When I visit "/pub/scm /linux/kernel/git/torvals/linux.git"
at git.kernel.org, for example, there currently are two links, "/pub/scm"
to the toplevel and "/linux/kernel/git/torvals/linux.git" to itself. I
often wish to see uplinks to intermediate levels like "/linux/kernel/git"
and "/linux/kernel/git/torvalds".

Perhaps that is the topic of your second patch. I dunno.

^ permalink raw reply

* Re: BUG 1.7.9: git branch fails to create new branch when --edit-description is used
From: Junio C Hamano @ 2012-01-29 21:49 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: Mark Jason Dominus, git
In-Reply-To: <4F24E78A.7060502@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

> On 01/28/2012 08:27 AM, Junio C Hamano wrote:
>> Michael Haggerty <mhagger@alum.mit.edu> writes:
>> 
> I vote for an error.  Otherwise a typo in the branch name would lead to
> the description's apparent disappearance into Nirvana.  An error would,
> for example, have made it clear to the OP what was happening.

I agree with this statement.

Unless or until we update --edit-description to create a new branch when
given a name of a branch that does not exist, the existing behaviour is
simply an accident waiting to happen.

^ permalink raw reply

* [PATCH/RFC v2] grep: Add the option '--exclude'
From: Albert Yale @ 2012-01-29 22:42 UTC (permalink / raw)
  To: git; +Cc: gitster, Albert Yale

Signed-off-by: Albert Yale <surfingalbert@gmail.com>
---
This is a revision to my previous patch.
The exclusion information is now held
inside the pathspec.

Feedback would again be appreciated,

Albert Yale

 Documentation/git-grep.txt |    7 +++++
 builtin/grep.c             |   43 ++++++++++++++++++++++++++++++++-
 cache.h                    |    3 ++
 dir.c                      |   55 ++++++++++++++++++++++++++++++++++++++++----
 4 files changed, 101 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 6a8b1e3..db143e3 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -22,6 +22,7 @@ SYNOPSIS
 	   [--color[=<when>] | --no-color]
 	   [-A <post-context>] [-B <pre-context>] [-C <context>]
 	   [-f <file>] [-e] <pattern>
+	   [-x<pattern>|--exclude<pattern>]
 	   [--and|--or|--not|(|)|-e <pattern>...]
 	   [ [--exclude-standard] [--cached | --no-index | --untracked] | <tree>...]
 	   [--] [<pathspec>...]
@@ -124,6 +125,12 @@ OPTIONS
 	Use fixed strings for patterns (don't interpret pattern
 	as a regex).
 
+-x<pattern>::
+--exclude<pattern>::
+	In addition to those found in .gitignore (per directory) and
+	$GIT_DIR/info/exclude, also consider these patterns to be in the
+	set of the ignore rules in effect.
+
 -n::
 --line-number::
 	Prefix the line number to matching lines.
diff --git a/builtin/grep.c b/builtin/grep.c
index 9ce064a..9772fa4 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -530,6 +530,10 @@ static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec, int
 			continue;
 		if (!match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, NULL))
 			continue;
+		if (pathspec->exclude &&
+			pathspec->exclude->nr &&
+			match_pathspec_depth(pathspec->exclude, ce->name, ce_namelen(ce), 0, NULL))
+			continue;
 		/*
 		 * If CE_VALID is on, we assume worktree file and its cache entry
 		 * are identical, even if worktree file has been modified, so use
@@ -566,6 +570,11 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
 	while (tree_entry(tree, &entry)) {
 		int te_len = tree_entry_len(&entry);
 
+		if (pathspec->exclude &&
+			pathspec->exclude->nr &&
+			match_pathspec_depth(pathspec->exclude, entry.path, strlen(entry.path), 0, NULL))
+			continue;
+
 		if (match != all_entries_interesting) {
 			match = tree_entry_interesting(&entry, base, tn_len, pathspec);
 			if (match == all_entries_not_interesting)
@@ -606,8 +615,16 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
 static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,
 		       struct object *obj, const char *name)
 {
-	if (obj->type == OBJ_BLOB)
+	if (obj->type == OBJ_BLOB) {
+		const char *name_without_sha1 = strchr(name, ':') + 1;
+
+		if (pathspec->exclude &&
+			pathspec->exclude->nr &&
+			match_pathspec_depth(pathspec->exclude, name_without_sha1, strlen(name_without_sha1), 0, NULL))
+			return 0;
+
 		return grep_sha1(opt, obj->sha1, name, 0);
+	}
 	if (obj->type == OBJ_COMMIT || obj->type == OBJ_TREE) {
 		struct tree_desc tree;
 		void *data;
@@ -673,6 +690,10 @@ static int grep_directory(struct grep_opt *opt, const struct pathspec *pathspec,
 		int namelen = strlen(name);
 		if (!match_pathspec_depth(pathspec, name, namelen, 0, NULL))
 			continue;
+		if (pathspec->exclude &&
+			pathspec->exclude->nr &&
+			match_pathspec_depth(pathspec->exclude, name, namelen, 0, NULL))
+			continue;
 		hit |= grep_file(opt, dir.entries[i]->name);
 		if (hit && opt->status_only)
 			break;
@@ -764,6 +785,14 @@ static int pattern_callback(const struct option *opt, const char *arg,
 	return 0;
 }
 
+static int exclude_cb(const struct option *opt, const char *arg,
+			    int unset)
+{
+	struct string_list *exclude_list = opt->value;
+	string_list_append(exclude_list, arg);
+	return 0;
+}
+
 static int help_callback(const struct option *opt, const char *arg, int unset)
 {
 	return -1;
@@ -792,7 +821,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		pattern_type_pcre,
 	};
 	int pattern_type = pattern_type_unspecified;
-
+	struct string_list exclude_list = STRING_LIST_INIT_NODUP;
 	struct option options[] = {
 		OPT_BOOLEAN(0, "cached", &cached,
 			"search in index instead of in the work tree"),
@@ -872,6 +901,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			"read patterns from file", file_callback),
 		{ OPTION_CALLBACK, 'e', NULL, &opt, "pattern",
 			"match <pattern>", PARSE_OPT_NONEG, pattern_callback },
+		{ OPTION_CALLBACK, 'x', "exclude", &exclude_list, "pattern",
+		  "add <pattern> to ignore rules", PARSE_OPT_NONEG, exclude_cb },
 		{ OPTION_CALLBACK, 0, "and", &opt, NULL,
 		  "combine patterns specified with -e",
 		  PARSE_OPT_NOARG | PARSE_OPT_NONEG, and_callback },
@@ -1053,6 +1084,12 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	pathspec.max_depth = opt.max_depth;
 	pathspec.recursive = 1;
 
+	if( exclude_list.nr ) {
+		create_pathspec_from_string_list(&pathspec.exclude, &exclude_list);
+		pathspec.exclude->max_depth = opt.max_depth;
+		pathspec.exclude->recursive = 1;
+	}
+
 	if (show_in_pager && (cached || list.nr))
 		die(_("--open-files-in-pager only works on the worktree"));
 
@@ -1102,5 +1139,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	if (hit && show_in_pager)
 		run_pager(&opt, prefix);
 	free_grep_patterns(&opt);
+	free_pathspec(&pathspec);
+	string_list_clear(&exclude_list, 0);
 	return !hit;
 }
diff --git a/cache.h b/cache.h
index 10afd71..882a390 100644
--- a/cache.h
+++ b/cache.h
@@ -533,9 +533,12 @@ struct pathspec {
 		int len;
 		unsigned int use_wildcard:1;
 	} *items;
+
+	struct pathspec *exclude;
 };
 
 extern int init_pathspec(struct pathspec *, const char **);
+extern int create_pathspec_from_string_list(struct pathspec **, const struct string_list *);
 extern void free_pathspec(struct pathspec *);
 extern int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec);
 
diff --git a/dir.c b/dir.c
index 0a78d00..fd04afa 100644
--- a/dir.c
+++ b/dir.c
@@ -9,6 +9,8 @@
 #include "dir.h"
 #include "refs.h"
 
+#include "string-list.h"
+
 struct path_simplify {
 	int len;
 	const char *path;
@@ -1259,6 +1261,49 @@ static int pathspec_item_cmp(const void *a_, const void *b_)
 	return strcmp(a->match, b->match);
 }
 
+static int init_pathspec_item( struct pathspec_item *item, const char *path )
+{
+	item->match = path;
+	item->len = strlen(path);
+	item->use_wildcard = !no_wildcard(path);
+
+	return item->use_wildcard;
+}
+
+int create_pathspec_from_string_list(struct pathspec **pathspec, const struct string_list *path_list)
+{
+	int i;
+
+	*pathspec = xcalloc( 1, sizeof( struct pathspec ) );
+
+	if (!path_list->nr)
+		return 0;
+
+	(*pathspec)->nr = path_list->nr;
+	(*pathspec)->items = xcalloc(path_list->nr, sizeof(struct pathspec_item));
+
+	for (i = 0; i < path_list->nr; i++) {
+		struct pathspec_item *item = (*pathspec)->items+i;
+		const char *path = path_list->items[i].string;
+
+		(*pathspec)->has_wildcard |= init_pathspec_item( item, path );
+	}
+
+	qsort((*pathspec)->items, (*pathspec)->nr,
+	      sizeof(struct pathspec_item), pathspec_item_cmp);
+
+	/*
+	* Please comment/review:
+	*
+	* Here, pathspec->raw is NULL despite pathspec->nr being non-zero.
+	* A possible problem might arrise if other areas of the code make
+	* the assumption that a NULL pathspec->raw means that pathspec->nr
+	* is zero.
+	*/
+
+	return 0;
+}
+
 int init_pathspec(struct pathspec *pathspec, const char **paths)
 {
 	const char **p = paths;
@@ -1279,11 +1324,7 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)
 		struct pathspec_item *item = pathspec->items+i;
 		const char *path = paths[i];
 
-		item->match = path;
-		item->len = strlen(path);
-		item->use_wildcard = !no_wildcard(path);
-		if (item->use_wildcard)
-			pathspec->has_wildcard = 1;
+		pathspec->has_wildcard |= init_pathspec_item( item, path );
 	}
 
 	qsort(pathspec->items, pathspec->nr,
@@ -1296,4 +1337,8 @@ void free_pathspec(struct pathspec *pathspec)
 {
 	free(pathspec->items);
 	pathspec->items = NULL;
+	if (pathspec->exclude) {
+		free_pathspec(pathspec->exclude);
+		pathspec->exclude = NULL;
+	}
 }
-- 
1.7.8.3

^ permalink raw reply related

* Re: [PATCH/RFC v2] grep: Add the option '--exclude'
From: Junio C Hamano @ 2012-01-29 23:02 UTC (permalink / raw)
  To: Albert Yale; +Cc: git
In-Reply-To: <1327876934-61526-1-git-send-email-surfingalbert@gmail.com>

Albert Yale <surfingalbert@gmail.com> writes:

> Feedback would again be appreciated,

Hmm.

>  	   [-f <file>] [-e] <pattern>
> +	   [-x<pattern>|--exclude<pattern>]

Compare the above two lines and notice some funny in the added one?

> @@ -124,6 +125,12 @@ OPTIONS
>  	Use fixed strings for patterns (don't interpret pattern
>  	as a regex).
>  
> +-x<pattern>::
> +--exclude<pattern>::
> +	In addition to those found in .gitignore (per directory) and
> +	$GIT_DIR/info/exclude, also consider these patterns to be in the
> +	set of the ignore rules in effect.
> +

Likewise.

> diff --git a/builtin/grep.c b/builtin/grep.c
> index 9ce064a..9772fa4 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -530,6 +530,10 @@ static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec, int
>  			continue;
>  		if (!match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, NULL))
>  			continue;
> +		if (pathspec->exclude &&
> +			pathspec->exclude->nr &&
> +			match_pathspec_depth(pathspec->exclude, ce->name, ce_namelen(ce), 0, NULL))
> +			continue;

Why isn't this just

	if (!match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, NULL))

that is, *no change whatsoever* on the existing codepaths that call
match_pathspec_depth()?  Can't the "even if one of the positive pathspec
matched, if one of the excluded one matches, declare that the path does
*not* match" logic live in match_pathspec_depth() itself?

Exactly the same comment applies to all the other additions that calls
match_pathspec_depth() with pathspec->exclude as its first parameter in
this patch.

> @@ -1053,6 +1084,12 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>  	pathspec.max_depth = opt.max_depth;
>  	pathspec.recursive = 1;
>  
> +	if( exclude_list.nr ) {
> +		create_pathspec_from_string_list(&pathspec.exclude, &exclude_list);
> +		pathspec.exclude->max_depth = opt.max_depth;
> +		pathspec.exclude->recursive = 1;
> +	}
> +

Style. Notice where SPs should be near parentheses on "if" statement in
our codebase.

^ permalink raw reply

* Re: [PATCH v4 1/2] gitweb: add project_filter to limit project list to a subdirectory
From: Jakub Narebski @ 2012-01-29 23:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Bernhard R. Link, git
In-Reply-To: <7v7h0afcc2.fsf@alter.siamese.dyndns.org>

On Sun, 29 Jan 2012, Junio C Hamano wrote:
> "Bernhard R. Link" <brl+git@mail.brlink.eu> writes:
> 
> > This commit changes the project listing views (project_list,
> > project_index and opml) to limit the output to only projects in a
> > subdirectory if the new optional parameter ?pf=directory name is used.
> >
> > The change is quite minimal as git_get_projects_list already can limit
> > itself to a subdirectory (though that was previously only used for
> > 'forks').
> >
> > If there is a GITWEB_LIST file, the contents are just filtered like
> > with the forks action.
> 
> Meaning, a directory is shown if it is listed on GITWEB_LIST and is a
> subdirectory of the directory specified with project_filter?  If so,
> spelling it out instead of saying "just filtered like with the forks
> action" may be clearer without making the description excessively longer.

This means the following:

  If $projects_list point to file with a list of projects, gitweb will
  show only those project on the list which name matches $project_filter
  prefix.

> > Without a GITWEB_LIST file only the given subdirectory is searched
> > for projects (like with forks) unless GITWEB_STRICT_EXPORT is enabled.
> > In the later case GITWEB_PROJECTROOT is traversed normally (unlike
> > with forks) and projects not in the directory ignored.
> 
> It is unclear to me what "In the later case" refers to, even assuming that
> it is a typo of "the latter case".
> 
> Do you mean "When there is no GITWEB_LIST but GITWEB_STRICT_EXPORT is set,
> project_filter that specifies anything outside GITWEB_PROJECTROOT is
> ignored"?

  If $projects_list points to a directory, but $strict_export is true, then
  $project_list is scanned recursively for git repositories, as without
  this feature, but only those projects that begin with $project_filter
  are shown.

  Otherwise ($projects_list points to directory and $strict_export not true)
  then $project_filter subdirectory of $projects_list is scanned recursively
  for git repositories (i.e. starting from "projects_list/$project_filter").
 
I am not sure if this paranoia mode is really needed for $strict_export,
though.

> A more fundamental issue I have with this patch is how an end user starts
> using this. Once project_filter is set, the breadcrumbs would let the user
> click and navigate around, but in my superficial glance at the patch it is
> not apparent how the initial setting of project_filter can happen without
> the user manually adding pf= to the URL, which is a less than ideal end
> user experience.

The second patch in series adds breadcrumbs allowing to filter projects
in any per-project view.

This feature was originally intended for giving handcrafter URL with
'pf=....' to people...

> > @@ -2839,7 +2848,7 @@ sub git_get_projects_list {
> >  		my $pfxlen = length("$dir");
> >  		my $pfxdepth = ($dir =~ tr!/!!);
> >  		# when filtering, search only given subdirectory
> > -		if ($filter) {
> > +		if ($filter and not $paranoid) {
> >  			$dir .= "/$filter";
> >  			$dir =~ s!/+$!!;
> >  		}
> > @@ -2864,6 +2873,10 @@ sub git_get_projects_list {
> >  				}
> >  
> >  				my $path = substr($File::Find::name, $pfxlen + 1);
> > +				# paranoidly only filter here
> > +				if ($paranoid && $filter && $path !~ m!^\Q$filter\E/!) {
> > +					next;
> > +				}
> 
> When you find "foo" directory and a project_filter tells you to match
> "foo", because $path does not match "^foo/", it will not match (even
> though its subdirectory "foo/bar" would)?

Strictly speaking the match is on dirname of a project path; the basename
of a project does not matter.  It is intended, but perhaps should be made
more clear in the commit message.
 
> > +sub print_nav_breadcrumbs_path {
> > +	my $dirprefix = undef;
> > +	while (my $part = shift) {
> > +		$dirprefix .= "/" if defined $dirprefix;
> > +		$dirprefix .= $part;
> > +		print $cgi->a({-href => href(project => undef,
> > +		                             project_filter => $dirprefix,
> > +					     action=>"project_list")},
> > +			      esc_html($part)) . " / ";
> > +	}
> > +}
> > +
> >  sub print_nav_breadcrumbs {
> >  	my %opts = @_;
> >  
> > @@ -3841,6 +3866,8 @@ sub print_nav_breadcrumbs {
> >  			print " / $opts{-action_extra}";
> >  		}
> >  		print "\n";
> > +	} elsif (defined $project_filter) {
> > +		print_nav_breadcrumbs_path(split '/', $project_filter);
> >  	}
> >  }
> 
> Hmm.
> 
> While this may not be wrong, I wonder if this is limiting a useful feature
> too narrowly. When I visit "/pub/scm /linux/kernel/git/torvals/linux.git"
> at git.kernel.org, for example, there currently are two links, "/pub/scm"
> to the toplevel and "/linux/kernel/git/torvals/linux.git" to itself. I
> often wish to see uplinks to intermediate levels like "/linux/kernel/git"
> and "/linux/kernel/git/torvalds".
> 
> Perhaps that is the topic of your second patch. I dunno.

Yes, it is.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: git-gui Ctrl-U (unstage) broken
From: Pat Thoyts @ 2012-01-29 23:19 UTC (permalink / raw)
  To: Victor Engmark; +Cc: git
In-Reply-To: <CAA5Ydx-mi7i7mWDYO=Cbw4g1b7LR0hw4Tcqe9gMtBoCkDRuvYA@mail.gmail.com>

Victor Engmark <victor.engmark@gmail.com> writes:

>Using the git-gui available with the default Ubuntu 10.10 repos, I'm
>not able to unstage files with the default keyboard shortcut. To
>reproduce:
>1. Change a file in the repository
>2. Run `git gui`
>3. Stage the changed file
>4. Select the changed file in the "Staged Changes (Will Commit)" list
>5. Click Ctrl-U
>
>Expected outcome: The selected file should be unstaged.
>
>Actual outcome: Nothing at all changes in the GUI.
>
>Verified that other keyboard shortcuts work: Ctrl-T, Ctrl-I, Ctrl--,
>Ctrl-+, F5. These (except Ctrl-T, obviously) were tested in* both the
>"Unstaged Changes" and "Staged Changes (Will Commit)" listsp
>
>* That is, after focusing a single element in that list.
>
>Version info:
>
>git-gui version 0.12.0.64.g89d6
>git version 1.7.1
>
>Tcl/Tk version 8.5.8
>Aspell 0.60.6, en_US

I checked this with the current version (gitgui-0.16.0) and it works ok
for me (on windows) - ie: ctrl-u unstaged a selected file.
-- 
Pat Thoyts                            http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD

^ permalink raw reply

* [PATCH 0/3] completion: trivial cleanups
From: Felipe Contreras @ 2012-01-29 23:41 UTC (permalink / raw)
  To: git; +Cc: Felipe Contreras

And an improvement for zsh.

Felipe Contreras (3):
  completion: be nicer with zsh
  completion: remove old code
  completion: remove unused code

 contrib/completion/git-completion.bash |   47 +++++---------------------------
 1 files changed, 7 insertions(+), 40 deletions(-)

-- 
1.7.8.3

^ permalink raw reply

* [PATCH 1/3] completion: be nicer with zsh
From: Felipe Contreras @ 2012-01-29 23:41 UTC (permalink / raw)
  To: git; +Cc: Felipe Contreras
In-Reply-To: <1327880479-25275-1-git-send-email-felipe.contreras@gmail.com>

And yet another bug in zsh[1] causes a mismatch; zsh seems to have
problem emulating wordspliting, but only when the ':' command is
involved.

Let's avoid it. This has the advantage that the code is now actually
understandable (at least to me), while before it looked like voodoo.

I found this issue because __git_compute_porcelain_commands listed all
commands (not only porcelain).

[1] http://article.gmane.org/gmane.comp.shells.zsh.devel/24296

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 1496c6d..7051c7a 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -676,7 +676,8 @@ __git_merge_strategies=
 # is needed.
 __git_compute_merge_strategies ()
 {
-	: ${__git_merge_strategies:=$(__git_list_merge_strategies)}
+	test "$__git_merge_strategies" && return
+	__git_merge_strategies=$(__git_list_merge_strategies 2> /dev/null)
 }
 
 __git_complete_revlist_file ()
@@ -854,7 +855,8 @@ __git_list_all_commands ()
 __git_all_commands=
 __git_compute_all_commands ()
 {
-	: ${__git_all_commands:=$(__git_list_all_commands)}
+	test "$__git_all_commands" && return
+	__git_all_commands=$(__git_list_all_commands 2> /dev/null)
 }
 
 __git_list_porcelain_commands ()
@@ -947,7 +949,8 @@ __git_porcelain_commands=
 __git_compute_porcelain_commands ()
 {
 	__git_compute_all_commands
-	: ${__git_porcelain_commands:=$(__git_list_porcelain_commands)}
+	test "$__git_porcelain_commands" && return
+	__git_porcelain_commands=$(__git_list_porcelain_commands 2> /dev/null)
 }
 
 __git_pretty_aliases ()
-- 
1.7.8.3

^ permalink raw reply related

* [PATCH 2/3] completion: remove old code
From: Felipe Contreras @ 2012-01-29 23:41 UTC (permalink / raw)
  To: git; +Cc: Felipe Contreras
In-Reply-To: <1327880479-25275-1-git-send-email-felipe.contreras@gmail.com>

We don't need to check for GIT_DIR/remotes, right? This was removed long
time ago.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash |    8 +-------
 1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 7051c7a..f7278b5 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -643,13 +643,7 @@ __git_refs_remotes ()
 
 __git_remotes ()
 {
-	local i ngoff IFS=$'\n' d="$(__gitdir)"
-	__git_shopt -q nullglob || ngoff=1
-	__git_shopt -s nullglob
-	for i in "$d/remotes"/*; do
-		echo ${i#$d/remotes/}
-	done
-	[ "$ngoff" ] && __git_shopt -u nullglob
+	local i IFS=$'\n' d="$(__gitdir)"
 	for i in $(git --git-dir="$d" config --get-regexp 'remote\..*\.url' 2>/dev/null); do
 		i="${i#remote.}"
 		echo "${i/.url*/}"
-- 
1.7.8.3

^ permalink raw reply related

* [PATCH 3/3] completion: remove unused code
From: Felipe Contreras @ 2012-01-29 23:41 UTC (permalink / raw)
  To: git; +Cc: Felipe Contreras
In-Reply-To: <1327880479-25275-1-git-send-email-felipe.contreras@gmail.com>

No need for thus rather complicated piece of code :)

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash |   30 ------------------------------
 1 files changed, 0 insertions(+), 30 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index f7278b5..59a4650 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2730,33 +2730,3 @@ if [ Cygwin = "$(uname -o 2>/dev/null)" ]; then
 complete -o bashdefault -o default -o nospace -F _git git.exe 2>/dev/null \
 	|| complete -o default -o nospace -F _git git.exe
 fi
-
-if [[ -n ${ZSH_VERSION-} ]]; then
-	__git_shopt () {
-		local option
-		if [ $# -ne 2 ]; then
-			echo "USAGE: $0 (-q|-s|-u) <option>" >&2
-			return 1
-		fi
-		case "$2" in
-		nullglob)
-			option="$2"
-			;;
-		*)
-			echo "$0: invalid option: $2" >&2
-			return 1
-		esac
-		case "$1" in
-		-q)	setopt | grep -q "$option" ;;
-		-u)	unsetopt "$option" ;;
-		-s)	setopt "$option" ;;
-		*)
-			echo "$0: invalid flag: $1" >&2
-			return 1
-		esac
-	}
-else
-	__git_shopt () {
-		shopt "$@"
-	}
-fi
-- 
1.7.8.3

^ permalink raw reply related

* What's cooking in git.git (Jan 2012, #07; Sun, 29)
From: Junio C Hamano @ 2012-01-29 23:48 UTC (permalink / raw)
  To: git

Here are the topics that have been cooking.  Commits prefixed with '-' are
only in 'pu' (proposed updates) while commits prefixed with '+' are in
'next'.

As promised in the recent couple of issues of "What's cooking", the first
batch of topics that have been cooking in 'next' are now in 'master'. The
tip of 'next' hasn't been rewound yet, but it soon will after the second
batch of topics graduate.

Here are the repositories that have my integration branches:

With maint, master, next, pu, todo:

        git://git.kernel.org/pub/scm/git/git.git
        git://repo.or.cz/alt-git.git
        https://code.google.com/p/git-core/
        https://github.com/git/git

With only maint and master:

        git://git.sourceforge.jp/gitroot/git-core/git.git
        git://git-core.git.sourceforge.net/gitroot/git-core/git-core

With all the topics and integration branches:

        https://github.com/gitster/git

The preformatted documentation in HTML and man format are found in:

        git://git.kernel.org/pub/scm/git/git-{htmldocs,manpages}.git/
        git://repo.or.cz/git-{htmldocs,manpages}.git/
        https://code.google.com/p/git-{htmldocs,manpages}.git/
        https://github.com/gitster/git-{htmldocs,manpages}.git/

--------------------------------------------------
[New Topics]

* bl/gitweb-project-filter (2012-01-29) 2 commits
 - gitweb: place links to parent directories in page header
 - gitweb: add project_filter to limit project list to a subdirectory

* rt/completion-branch-edit-desc (2012-01-29) 1 commit
 - completion: --edit-description option for git-branch

--------------------------------------------------
[Graduated to "master"]

* cb/git-daemon-tests (2012-01-08) 5 commits
  (merged to 'next' on 2012-01-08 at 1db8351)
 + git-daemon tests: wait until daemon is ready
 + git-daemon: produce output when ready
 + git-daemon: add tests
 + dashed externals: kill children on exit
 + run-command: optionally kill children on exit

* cb/push-quiet (2012-01-08) 3 commits
  (merged to 'next' on 2012-01-20 at 4326dda)
 + t5541: avoid TAP test miscounting
 + fix push --quiet: add 'quiet' capability to receive-pack
 + server_supports(): parse feature list more carefully

* jc/maint-log-first-parent-pathspec (2012-01-19) 1 commit
  (merged to 'next' on 2012-01-20 at fb2b35f)
 + Making pathspec limited log play nicer with --first-parent

* jk/parse-object-cached (2012-01-06) 3 commits
  (merged to 'next' on 2012-01-08 at 8c6fa4a)
 + upload-pack: avoid parsing tag destinations
 + upload-pack: avoid parsing objects during ref advertisement
 + parse_object: try internal cache before reading object db

These are a bit scary changes, but I do think they are worth doing.

* jl/test-pause (2012-01-17) 1 commit
  (merged to 'next' on 2012-01-20 at ee56335)
 + test-lib: add the test_pause convenience function

* jn/gitweb-unspecified-action (2012-01-09) 1 commit
  (merged to 'next' on 2012-01-20 at 2b31714)
 + gitweb: Fix actionless dispatch for non-existent objects

* mh/ref-clone-without-extra-refs (2012-01-17) 4 commits
  (merged to 'next' on 2012-01-20 at 2e9645e)
 + write_remote_refs(): create packed (rather than extra) refs
 + add_packed_ref(): new function in the refs API.
 + ref_array: keep track of whether references are sorted
 + pack_refs(): remove redundant check

Looked reasonable; will hopefully help making mh/ref-api-rest simpler and
cleaner.

* nd/clone-single-branch (2012-01-08) 1 commit
  (merged to 'next' on 2012-01-09 at 6c3c759)
 + clone: add --single-branch to fetch only one branch
 (this branch is used by nd/clone-detached.)

* nd/index-pack-no-recurse (2012-01-16) 3 commits
  (merged to 'next' on 2012-01-20 at d1e964e)
 + index-pack: eliminate unlimited recursion in get_base_data()
 + index-pack: eliminate recursion in find_unresolved_deltas
 + Eliminate recursion in setting/clearing marks in commit list

* nd/maint-refname-in-hierarchy-check (2012-01-11) 1 commit
  (merged to 'next' on 2012-01-20 at acb5611)
 + Fix incorrect ref namespace check

* pw/p4-view-updates (2012-01-11) 5 commits
  (merged to 'next' on 2012-01-20 at 8ca2c7b)
 + git-p4: add tests demonstrating spec overlay ambiguities
 + git-p4: adjust test to adhere to stricter useClientSpec
 + git-p4: clarify comment
 + git-p4: fix verbose comment typo
 + git-p4: only a single ... wildcard is supported

* rs/diff-postimage-in-context (2012-01-06) 1 commit
  (merged to 'next' on 2012-01-09 at 9635032)
 + xdiff: print post-image for common records instead of pre-image

* sp/smart-http-failure-to-push (2012-01-20) 1 commit
  (merged to 'next' on 2012-01-20 at a892434)
 + remote-curl: Fix push status report when all branches fail

* tr/maint-mailinfo (2012-01-16) 2 commits
  (merged to 'next' on 2012-01-20 at 278fae1)
 + mailinfo: with -b, keep space after [foo]
 + am: learn passing -b to mailinfo

--------------------------------------------------
[Stalled]

* jc/advise-push-default (2011-12-18) 1 commit
 - push: hint to use push.default=upstream when appropriate

Peff had a good suggestion outlining an updated code structure so that
somebody new can try to dip his or her toes in the development. Any
takers?

Waiting for a reroll.

* ss/git-svn-prompt-sans-terminal (2012-01-04) 3 commits
 - fixup! 15eaaf4
 - git-svn, perl/Git.pm: extend Git::prompt helper for querying users
  (merged to 'next' on 2012-01-05 at 954f125)
 + perl/Git.pm: "prompt" helper to honor GIT_ASKPASS and SSH_ASKPASS

The bottom one has been replaced with a rewrite based on comments from
Ævar. The second one needs more work, both in perl/Git.pm and prompt.c, to
give precedence to tty over SSH_ASKPASS when terminal is available.

Will defer till the next cycle.

* nd/commit-ignore-i-t-a (2012-01-16) 2 commits
 - commit, write-tree: allow to ignore CE_INTENT_TO_ADD while writing trees
 - cache-tree: update API to take abitrary flags

May want to consider this as fixing an earlier UI mistake, and not as a
feature that devides the userbase.

Will defer till the next cycle.

--------------------------------------------------
[Cooking]

* jl/submodule-re-add (2012-01-24) 1 commit
  (merged to 'next' on 2012-01-26 at 482553e)
 + submodule add: fix breakage when re-adding a deep submodule

"git submodule add" forgot to recompute the name to be stored in .gitmodules
when the module was once added to the superproject and already initialized.

Will merge to 'master' in the second batch.

* jn/svn-fe (2012-01-27) 44 commits
  (merged to 'next' on 2012-01-29 at 001a395)
 + vcs-svn/svndiff.c: squelch false "unused" warning from gcc
 + Merge branch 'svn-fe' of git://repo.or.cz/git/jrn into jn/svn-fe
 + vcs-svn: reset first_commit_done in fast_export_init
 + Merge branch 'db/text-delta' into svn-fe
 + vcs-svn: do not initialize report_buffer twice
 + Merge branch 'db/text-delta' into svn-fe
 + vcs-svn: avoid hangs from corrupt deltas
 + vcs-svn: guard against overflow when computing preimage length
 + Merge branch 'db/delta-applier' into db/text-delta
 + vcs-svn: implement text-delta handling
 + Merge branch 'db/delta-applier' into db/text-delta
 + Merge branch 'db/delta-applier' into svn-fe
 + vcs-svn: cap number of bytes read from sliding view
 + test-svn-fe: split off "test-svn-fe -d" into a separate function
 + vcs-svn: let deltas use data from preimage
 + vcs-svn: let deltas use data from postimage
 + vcs-svn: verify that deltas consume all inline data
 + vcs-svn: implement copyfrom_data delta instruction
 + vcs-svn: read instructions from deltas
 + vcs-svn: read inline data from deltas
 + vcs-svn: read the preimage when applying deltas
 + vcs-svn: parse svndiff0 window header
 + vcs-svn: skeleton of an svn delta parser
 + vcs-svn: make buffer_read_binary API more convenient
 + vcs-svn: learn to maintain a sliding view of a file
 + Makefile: list one vcs-svn/xdiff object or header per line
 + Merge branch 'db/svn-fe-code-purge' into svn-fe
 + vcs-svn: drop obj_pool
 + vcs-svn: drop treap
 + vcs-svn: drop string_pool
 + vcs-svn: pass paths through to fast-import
 + Merge branch 'db/strbufs-for-metadata' into db/svn-fe-code-purge
 + Merge branch 'db/length-as-hash' (early part) into db/svn-fe-code-purge
 + Merge branch 'db/vcs-svn-incremental' into svn-fe
 + vcs-svn: avoid using ls command twice
 + vcs-svn: use mark from previous import for parent commit
 + vcs-svn: handle filenames with dq correctly
 + vcs-svn: quote paths correctly for ls command
 + vcs-svn: eliminate repo_tree structure
 + vcs-svn: add a comment before each commit
 + vcs-svn: save marks for imported commits
 + vcs-svn: use higher mark numbers for blobs
 + vcs-svn: set up channel to read fast-import cat-blob response
 + Merge commit 'v1.7.5' into svn-fe

"vcs-svn"/"svn-fe" learned to read dumps with svn-deltas and support
incremental imports.

Will merge to 'master' in the second batch.

* jc/split-blob (2012-01-24) 6 commits
 - chunked-object: streaming checkout
 - chunked-object: fallback checkout codepaths
 - bulk-checkin: support chunked-object encoding
 - bulk-checkin: allow the same data to be multiply hashed
 - new representation types in the packstream
 - varint-in-pack: refactor varint encoding/decoding

Not ready.

I finished the streaming checkout codepath, but as explained in 127b177
(bulk-checkin: support chunked-object encoding, 2011-11-30), these are
still early steps of a long and painful journey. At least pack-objects and
fsck need to learn the new encoding for the series to be usable locally,
and then index-pack/unpack-objects needs to learn it to be used remotely.

Given that I heard a lot of noise that people want large files, and that I
was asked by somebody at GitTogether'11 privately for an advice on how to
pay developers (not me) to help adding necessary support, I am somewhat
dissapointed that the original patch series that was sent almost two
months ago still remains here without much comments and updates from the
developer community. I even made the interface to the logic that decides
where to split chunks easily replaceable, and I deliberately made the
logic in the original patch extremely stupid to entice others, especially
the "bup" fanboys, to come up with a better logic, thinking that giving
people an easy target to shoot for, they may be encouraged to help
out. The plan is not working :-(.

* ar/i18n-no-gettext (2012-01-27) 4 commits
  (merged to 'next' on 2012-01-27 at 0ecf258)
 + i18n: Do not force USE_GETTEXT_SCHEME=fallthrough on NO_GETTEXT
  (merged to 'next' on 2012-01-23 at 694a94e)
 + i18n: Make NO_GETTEXT imply fallthrough scheme in shell l10n
 + add a Makefile switch to avoid gettext translation in shell scripts
 + git-sh-i18n: restructure the logic to compute gettext.sh scheme

Will merge to 'master' in the second batch and deal with any fallout in 'master'.

* da/maint-mergetool-twoway (2012-01-23) 1 commit
  (merged to 'next' on 2012-01-23 at f927323)
 + mergetool: Provide an empty file when needed

Caters to GUI merge backends that cannot merge two files without
a base by giving them an empty file as a "pretend" common ancestor.

Will merge to 'master' in the second batch and deal with any fallout in 'master'.

* ld/git-p4-branches-and-labels (2012-01-20) 5 commits
  (merged to 'next' on 2012-01-23 at 9020ec4)
 + git-p4: label import fails with multiple labels at the same changelist
 + git-p4: add test for p4 labels
 + git-p4: importing labels should cope with missing owner
 + git-p4: cope with labels with empty descriptions
 + git-p4: handle p4 branches and labels containing shell chars
 (this branch is used by va/git-p4-branch.)

Will merge to 'master' in the second batch.

* va/git-p4-branch (2012-01-26) 4 commits
  (merged to 'next' on 2012-01-26 at e67c52a)
 + t9801: do not overuse test_must_fail
 + git-p4: Change p4 command invocation
 + git-p4: Add test case for complex branch import
 + git-p4: Search for parent commit on branch creation
 (this branch uses ld/git-p4-branches-and-labels.)

Rerolled and Acked.
Will merge to 'master' in the second batch.

* ks/sort-wildcard-in-makefile (2012-01-22) 1 commit
  (merged to 'next' on 2012-01-23 at e2e0c1d)
 + t/Makefile: Use $(sort ...) explicitly where needed

t/Makefile is adjusted to prevent newer versions of GNU make from running
tests in seemingly random order.

Will merge to 'master' in the second batch.

* tr/grep-l-with-decoration (2012-01-23) 1 commit
  (merged to 'next' on 2012-01-23 at 42b8795)
 + grep: fix -l/-L interaction with decoration lines

Using "git grep -l/-L" together with options -W or --break may not make
much sense as the output is to only count the number of hits and there is
no place for file breaks, but the latter options made "-l/-L" to miscount
the hits.

Will merge to 'master' in the second batch.

* jc/pull-signed-tag (2012-01-23) 1 commit
  (merged to 'next' on 2012-01-23 at 4257553)
 + merge: use editor by default in interactive sessions

Per Linus's strong suggestion, sugarcoated (aka "taking blame for the
original UI screw-ups") so that it is easier for me to swallow and accept
a potentially huge backward incompatibility issue, "git merge" is made to
launch an editor to explain the merge in the merge commit by default in
interactive sessions.

I've updated the special-case environment variable to MERGE_AUTOEDIT that
scripts can set to "no" when they start. There is no plan to encourage
humans to keep using the historical behaviour, hence there is no support
for configuration variable (e.g. merge.autoedit) that can be set to 'no'.
Oh, also I updated the documentation a bit.

"git merge" in an interactive session learned to spawn the editor by
default to let the user edit the auto-generated merge message, to
encourage people to explain their merges better.

Will merge to 'master' in the second batch and deal with any fallout in 'master'.

* jc/advise-i18n (2011-12-22) 1 commit
  (merged to 'next' on 2012-01-23 at 6447013)
 + i18n of multi-line advice messages

Allow localization of advice messages that tend to be longer and
multi-line formatted. For now this is deliberately limited to advise()
interface and not vreportf() in general as touching the latter has
interactions with error() that has plumbing callers whose prefix "error: "
should never be translated.

Will merge to 'master' in the second batch.

* rr/sequencer (2012-01-11) 2 commits
  (merged to 'next' on 2012-01-23 at f349b56)
 + sequencer: factor code out of revert builtin
 + revert: prepare to move replay_action to header

Moving large chunk of code out of cherry-pick/revert for later reuse,
primarily to prepare for the next cycle.

Will merge to 'master' in the second batch.

* nd/clone-detached (2012-01-24) 12 commits
  (merged to 'next' on 2012-01-26 at 7b0cc8a)
 + clone: fix up delay cloning conditions
  (merged to 'next' on 2012-01-23 at bee31c6)
 + push: do not let configured foreign-vcs permanently clobbered
  (merged to 'next' on 2012-01-23 at 9cab64e)
 + clone: print advice on checking out detached HEAD
 + clone: allow --branch to take a tag
 + clone: refuse to clone if --branch points to bogus ref
 + clone: --branch=<branch> always means refs/heads/<branch>
 + clone: delay cloning until after remote HEAD checking
 + clone: factor out remote ref writing
 + clone: factor out HEAD update code
 + clone: factor out checkout code
 + clone: write detached HEAD in bare repositories
 + t5601: add missing && cascade

"git clone" learned to detach the HEAD in the resulting repository when
the source repository's HEAD does not point to a branch.

Will merge to 'master' in the second batch and deal with any fallout in 'master'.

--------------------------------------------------
[Discarded]

* mh/ref-api-rest (2011-12-12) 35 commits
 . repack_without_ref(): call clear_packed_ref_cache()
 . read_packed_refs(): keep track of the directory being worked in
 . is_refname_available(): query only possibly-conflicting references
 . refs: read loose references lazily
 . read_loose_refs(): take a (ref_entry *) as argument
 . struct ref_dir: store a reference to the enclosing ref_cache
 . sort_ref_dir(): take (ref_entry *) instead of (ref_dir *)
 . do_for_each_ref_in_dir*(): take (ref_entry *) instead of (ref_dir *)
 . add_entry(): take (ref_entry *) instead of (ref_dir *)
 . search_ref_dir(): take (ref_entry *) instead of (ref_dir *)
 . find_containing_direntry(): use (ref_entry *) instead of (ref_dir *)
 . add_ref(): take (ref_entry *) instead of (ref_dir *)
 . read_packed_refs(): take (ref_entry *) instead of (ref_dir *)
 . find_ref(): take (ref_entry *) instead of (ref_dir *)
 . is_refname_available(): take (ref_entry *) instead of (ref_dir *)
 . get_loose_refs(): return (ref_entry *) instead of (ref_dir *)
 . get_packed_refs(): return (ref_entry *) instead of (ref_dir *)
 . refs: wrap top-level ref_dirs in ref_entries
 . get_ref_dir(): keep track of the current ref_dir
 . do_for_each_ref(): only iterate over the subtree that was requested
 . refs: sort ref_dirs lazily
 . sort_ref_dir(): do not sort if already sorted
 . refs: store references hierarchically
 . refs.c: rename ref_array -> ref_dir
 . struct ref_entry: nest the value part in a union
 . check_refname_component(): return 0 for zero-length components
 . free_ref_entry(): new function
 . refs.c: reorder definitions more logically
 . is_refname_available(): reimplement using do_for_each_ref_in_array()
 . names_conflict(): simplify implementation
 . names_conflict(): new function, extracted from is_refname_available()
 . repack_without_ref(): reimplement using do_for_each_ref_in_array()
 . do_for_each_ref_in_arrays(): new function
 . do_for_each_ref_in_array(): new function
 . do_for_each_ref(): correctly terminate while processesing extra_refs

Will be re-rolled. Discarded without prejudice.

* mm/zsh-completion-regression-fix (2012-01-17) 1 commit
  (merged to 'next' on 2012-01-23 at 7bc2e0a)
 + bash-completion: don't add quoted space for ZSH (fix regression)

Superseded by a better fix already in 'master'.

^ permalink raw reply

* Re: [PATCH] grep: fix -l/-L interaction with decoration lines
From: Junio C Hamano @ 2012-01-29 23:49 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Albert Yale, René Scharfe, git
In-Reply-To: <74777e0e8633d980fee9a1a680a63535be042fdc.1327340917.git.trast@student.ethz.ch>

Thanks, makes sense.

^ permalink raw reply

* Re: [PATCH/RFC v2] grep: Add the option '--exclude'
From: Albert Yale @ 2012-01-30  0:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vvcnudsda.fsf@alter.siamese.dyndns.org>

Thank you for your feedback Junio.
I'll revise my patch next weekend.

Albert Yale

On Sun, Jan 29, 2012 at 6:02 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Albert Yale <surfingalbert@gmail.com> writes:
>
> > Feedback would again be appreciated,
>
> Hmm.
>
> >          [-f <file>] [-e] <pattern>
> > +        [-x<pattern>|--exclude<pattern>]
>
> Compare the above two lines and notice some funny in the added one?
>
> > @@ -124,6 +125,12 @@ OPTIONS
> >       Use fixed strings for patterns (don't interpret pattern
> >       as a regex).
> >
> > +-x<pattern>::
> > +--exclude<pattern>::
> > +     In addition to those found in .gitignore (per directory) and
> > +     $GIT_DIR/info/exclude, also consider these patterns to be in the
> > +     set of the ignore rules in effect.
> > +
>
> Likewise.
>
> > diff --git a/builtin/grep.c b/builtin/grep.c
> > index 9ce064a..9772fa4 100644
> > --- a/builtin/grep.c
> > +++ b/builtin/grep.c
> > @@ -530,6 +530,10 @@ static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec, int
> >                       continue;
> >               if (!match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, NULL))
> >                       continue;
> > +             if (pathspec->exclude &&
> > +                     pathspec->exclude->nr &&
> > +                     match_pathspec_depth(pathspec->exclude, ce->name, ce_namelen(ce), 0, NULL))
> > +                     continue;
>
> Why isn't this just
>
>        if (!match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, NULL))
>
> that is, *no change whatsoever* on the existing codepaths that call
> match_pathspec_depth()?  Can't the "even if one of the positive pathspec
> matched, if one of the excluded one matches, declare that the path does
> *not* match" logic live in match_pathspec_depth() itself?
>
> Exactly the same comment applies to all the other additions that calls
> match_pathspec_depth() with pathspec->exclude as its first parameter in
> this patch.
>
> > @@ -1053,6 +1084,12 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
> >       pathspec.max_depth = opt.max_depth;
> >       pathspec.recursive = 1;
> >
> > +     if( exclude_list.nr ) {
> > +             create_pathspec_from_string_list(&pathspec.exclude, &exclude_list);
> > +             pathspec.exclude->max_depth = opt.max_depth;
> > +             pathspec.exclude->recursive = 1;
> > +     }
> > +
>
> Style. Notice where SPs should be near parentheses on "if" statement in
> our codebase.

^ permalink raw reply

* [PATCH] completion: add new zsh completion
From: Felipe Contreras @ 2012-01-30  0:01 UTC (permalink / raw)
  To: git; +Cc: Felipe Contreras

It seems there's always issues with zsh's bash completion emulation,
after I took a deep look at the code, I found many issues[1].

So, here is a kind of wrapper that does the same, but properly :)

This would also allow us to make fixes if necessary, since the code is
simple enough, and extend functionality.

[1] http://article.gmane.org/gmane.comp.shells.zsh.devel/24290

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.zsh |  146 +++++++++++++++++++++++++++++++++
 1 files changed, 146 insertions(+), 0 deletions(-)
 create mode 100644 contrib/completion/git-completion.zsh

diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh
new file mode 100644
index 0000000..15961df
--- /dev/null
+++ b/contrib/completion/git-completion.zsh
@@ -0,0 +1,146 @@
+#compdef git gitk
+
+compgen () {
+	local prefix suffix
+	local -a results
+	while getopts "W:P:S:" opt
+	do
+		case $opt in
+		W)
+			results=( ${(Q)~=OPTARG} )
+			;;
+		P)
+			prefix="$OPTARG"
+			;;
+		S)
+			suffix="$OPTARG"
+			;;
+		esac
+	done
+	print -l -r -- "$prefix${^results[@]}$suffix"
+}
+
+complete () {
+	# do nothing
+	return 0
+}
+
+ZSH_VERSION='' . /usr/share/git/completion/git-completion.bash
+
+_get_comp_words_by_ref ()
+{
+	while [ $# -gt 0 ]; do
+		case "$1" in
+		cur)
+			cur=${_words[CURRENT]}
+			;;
+		prev)
+			cur=${_words[CURRENT-1]}
+			;;
+		words)
+			words=("${_words[@]}")
+			;;
+		cword)
+			((cword = CURRENT - 1))
+			;;
+		esac
+		shift
+	done
+}
+
+_bash_wrap ()
+{
+	local -a COMPREPLY results _words
+	_words=( $words )
+	() {
+		emulate -L sh
+		setopt kshglob noshglob braceexpand nokshautoload
+		typeset -h words
+		local cur words cword prev
+		_get_comp_words_by_ref -n =: cur words cword prev
+		$1
+	} $1
+	results=( "${^COMPREPLY[@]}" )
+	local COMP_WORDBREAKS="\"'@><=;|&(:"
+	local i start
+	local cur="${words[CURRENT]}"
+	i=$(expr index "$cur" "$COMP_WORDBREAKS")
+	start="${cur:0:$i}"
+	compadd -Q -S '' -p "$start" -a results && return 0
+}
+
+_gitk ()
+{
+	__git_has_doubledash && return
+
+	local g="$(__gitdir)"
+	local merge=""
+	if [ -f "$g/MERGE_HEAD" ]; then
+		merge="--merge"
+	fi
+	case "$cur" in
+	--*)
+		__gitcomp "
+		$__git_log_common_options
+		$__git_log_gitk_options
+		$merge
+		"
+		return
+		;;
+	esac
+	__git_complete_revlist
+}
+
+_git ()
+{
+	if [[ $service != git ]]
+	then
+		_bash_wrap _$service
+		return ret
+	fi
+
+	local ret=1
+	local curcontext="$curcontext" state state_descr line
+	typeset -A opt_args
+
+	_arguments -C \
+		'(-p --paginate --no-pager)'{-p,--paginate}'[Pipe all output into ''less'']' \
+		'(-p --paginate)--no-pager[Do not pipe git output into a pager]' \
+		'--git-dir=-[Set the path to the repository]: :_directories' \
+		'--bare[Treat the repository as a bare repository]' \
+		'(- :)--version[Prints the git suite version]' \
+		'--exec-path=-[Path to where your core git programs are installed]:: :_directories' \
+		'--html-path[Print the path where git''s HTML documentation is installed]' \
+		'--work-tree=-[Set the path to the working tree]: :_directories' \
+		'--namespace=-[Set the git namespace]: :_directories' \
+		'(- :)--help[Prints the synopsis and a list of the most commonly used commands]' \
+		'(-): :->command' \
+		'(-)*:: :->option-or-argument' && return 0
+
+	case $state in
+	(command)
+		emulate sh -c __git_compute_porcelain_commands
+		local -a porcelain aliases
+		porcelain=( ${=__git_porcelain_commands} )
+		aliases=( $(__git_aliases) )
+		_describe -t porcelain-commands 'porcelain commands' porcelain && ret=0
+		_describe -t aliases 'aliases' aliases && ret=0
+		;;
+	(option-or-argument)
+		local command="${words[1]}"
+
+		completion_func="_git_${command//-/_}"
+		declare -f $completion_func >/dev/null && _bash_wrap $completion_func && return 0
+
+		local expansion=$(__git_aliased_command "$command")
+		if [ -n "$expansion" ]; then
+			completion_func="_git_${expansion//-/_}"
+			declare -f $completion_func >/dev/null && _bash_wrap $completion_func && return 0
+		fi
+		;;
+	esac
+
+	return ret
+}
+
+_git
-- 
1.7.8.3

^ permalink raw reply related

* [PATCH] completion: cleanup __gitcomp*
From: Felipe Contreras @ 2012-01-30  0:29 UTC (permalink / raw)
  To: git; +Cc: Felipe Contreras

I don't know why there's so much code; these functions don't seem to be
doing much:

 * ${1-} is the same as $1
 * no need to check $#, ${3:-$cur} is much easier
 * __gitcomp_nl doesn't seem to using the initial IFS

This makes the code much simpler.

Eventually it would be nice to wrap everything that touches compgen and
COMPREPLY in one function for the zsh wrapper.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash |   24 +++++-------------------
 1 files changed, 5 insertions(+), 19 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 1496c6d..accfce5 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -495,19 +495,15 @@ fi
 # 4: A suffix to be appended to each possible completion word (optional).
 __gitcomp ()
 {
-	local cur_="$cur"
-
-	if [ $# -gt 2 ]; then
-		cur_="$3"
-	fi
+	local cur_="${3:-$cur}"
 	case "$cur_" in
 	--*=)
 		COMPREPLY=()
 		;;
 	*)
 		local IFS=$'\n'
-		COMPREPLY=($(compgen -P "${2-}" \
-			-W "$(__gitcomp_1 "${1-}" "${4-}")" \
+		COMPREPLY=($(compgen -P "$2" \
+			-W "$(__gitcomp_1 "$1" "$4")" \
 			-- "$cur_"))
 		;;
 	esac
@@ -524,18 +520,8 @@ __gitcomp ()
 #    appended.
 __gitcomp_nl ()
 {
-	local s=$'\n' IFS=' '$'\t'$'\n'
-	local cur_="$cur" suffix=" "
-
-	if [ $# -gt 2 ]; then
-		cur_="$3"
-		if [ $# -gt 3 ]; then
-			suffix="$4"
-		fi
-	fi
-
-	IFS=$s
-	COMPREPLY=($(compgen -P "${2-}" -S "$suffix" -W "$1" -- "$cur_"))
+	local IFS=$'\n'
+	COMPREPLY=($(compgen -P "$2" -S "${4:- }" -W "$1" -- "${3:-$cur}"))
 }
 
 __git_heads ()
-- 
1.7.8.3

^ permalink raw reply related


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