Re: [PATCH 17/21] list-files: show directories as well as files

public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Junio C Hamano <gitster@pobox•com>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH 17/21] list-files: show directories as well as files
Date: Tue, 27 Jan 2015 13:51:29 -0800	[thread overview]
Message-ID: <xmqqmw53ki3i.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1422189476-7518-18-git-send-email-pclouds@gmail.com> ("Nguyễn	Thái Ngọc Duy"'s message of "Sun, 25 Jan 2015 19:37:52 +0700")

Nguyễn Thái Ngọc Duy  <pclouds@gmail•com> writes:

> @@ -194,16 +225,31 @@ static void write_ce_name(struct strbuf *sb, const struct cache_entry *ce)
>  static void show_ce_entry(const char *tag, const struct cache_entry *ce)
>  {
>  	static struct strbuf sb = STRBUF_INIT;
> -	int len = max_prefix_len;
> +	int len = max_prefix_len, saved_max_depth;
>  
>  	if (len >= ce_namelen(ce))
>  		die("git ls-files: internal error - cache entry not superset of prefix");
>  
> +	if (show_dirs) {
> +		/* ignore depth to catch dirs that contain matched entries */
> +		saved_max_depth = pathspec.max_depth;
> +		pathspec.max_depth = -1;
> +	}
> +
>  	if (!match_pathspec(&pathspec, ce->name, ce_namelen(ce),
>  			    len, ps_matched,
>  			    S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode)))
>  		return;
>  
> +	if (show_dirs) {
> +		pathspec.max_depth = saved_max_depth;
> +		if (strchr(ce->name, '/') &&
> +		    !match_pathspec(&pathspec, ce->name, ce_namelen(ce),
> +				    prefix_len, NULL, 1) &&
> +		    show_as_directory(ce))
> +			return;
> +	}
> +

My compiler seems to be too stupid to notice that saved_max_depth is
always set before it is used, if it gets used and complains.  Sigh.

For now I am tempted to squash this in.  Note that the original does
not seem to restore saved_max_depath when the pathspec does not match
and function returns in the call to match_pathspec() we have in the
code before your patch, which smells like a bug, and the attached
would fix it.

 builtin/ls-files.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 29b5c2e..f28b7e9 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -222,27 +222,38 @@ static void write_ce_name(struct strbuf *sb, const struct cache_entry *ce)
 	strbuf_release(&quoted);
 }
 
+static int match_pathspec_with_depth(struct pathspec *ps,
+				     const char *name, int namelen,
+				     int prefix, char *seen, int is_dir,
+				     const int *custom_depth)
+{
+	int saved_depth = ps->max_depth;
+	int result;
+
+	if (custom_depth)
+		ps->max_depth = *custom_depth;
+	result = match_pathspec(ps, name, namelen, prefix, seen, is_dir);
+	if (custom_depth)
+		ps->max_depth = saved_depth;
+	return result;
+}
+
 static void show_ce_entry(const char *tag, const struct cache_entry *ce)
 {
 	static struct strbuf sb = STRBUF_INIT;
-	int len = max_prefix_len, saved_max_depth;
+	int len = max_prefix_len;
+	static const int infinite_depth = -1;
 
 	if (len >= ce_namelen(ce))
 		die("git ls-files: internal error - cache entry not superset of prefix");
 
-	if (show_dirs) {
-		/* ignore depth to catch dirs that contain matched entries */
-		saved_max_depth = pathspec.max_depth;
-		pathspec.max_depth = -1;
-	}
-
-	if (!match_pathspec(&pathspec, ce->name, ce_namelen(ce),
-			    len, ps_matched,
-			    S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode)))
+	if (!match_pathspec_with_depth(&pathspec, ce->name, ce_namelen(ce),
+				       len, ps_matched,
+				       S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode),
+				       show_dirs ? &infinite_depth : NULL))
 		return;
 
 	if (show_dirs) {
-		pathspec.max_depth = saved_max_depth;
 		if (strchr(ce->name, '/') &&
 		    !match_pathspec(&pathspec, ce->name, ce_namelen(ce),
 				    prefix_len, NULL, 1) &&

next prev parent reply	other threads:[~2015-01-27 21:51 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-25 12:37 [PATCH 00/21] nd/list-files updates Nguyễn Thái Ngọc Duy
2015-01-25 12:37 ` [PATCH 01/21] ls_colors.c: add $LS_COLORS parsing code Nguyễn Thái Ngọc Duy
2015-01-25 12:37 ` [PATCH 02/21] ls_colors.c: parse color.ls.* from config file Nguyễn Thái Ngọc Duy
2015-01-25 12:37 ` [PATCH 03/21] ls_colors.c: add a function to color a file name Nguyễn Thái Ngọc Duy
2015-01-25 12:37 ` [PATCH 04/21] ls_colors.c: highlight submodules like directories Nguyễn Thái Ngọc Duy
2015-01-25 12:37 ` [PATCH 05/21] ls-files: buffer full item in strbuf before printing Nguyễn Thái Ngọc Duy
2015-01-25 12:37 ` [PATCH 06/21] ls-files: add --color to highlight file names Nguyễn Thái Ngọc Duy
2015-01-25 12:37 ` [PATCH 07/21] ls-files: add --column Nguyễn Thái Ngọc Duy
2015-01-25 12:37 ` [PATCH 08/21] ls-files: support --max-depth Nguyễn Thái Ngọc Duy
2015-01-25 12:37 ` [PATCH 09/21] list-files: a user friendly version of ls-files and more Nguyễn Thái Ngọc Duy
2015-01-27 20:30   ` Junio C Hamano
2015-01-25 12:37 ` [PATCH 10/21] list-files: make alias 'ls' default to 'list-files' Nguyễn Thái Ngọc Duy
2015-01-25 12:37 ` [PATCH 11/21] list-files: -u does not imply showing stages Nguyễn Thái Ngọc Duy
2015-01-25 12:37 ` [PATCH 12/21] list-files: add -R/--recursive short for --max-depth=-1 Nguyễn Thái Ngọc Duy
2015-01-25 12:37 ` [PATCH 13/21] list-files: add -1 short for --no-column Nguyễn Thái Ngọc Duy
2015-01-25 12:37 ` [PATCH 14/21] list-files: add -t back Nguyễn Thái Ngọc Duy
2015-01-25 12:37 ` [PATCH 15/21] list-files: sort output and remove duplicates Nguyễn Thái Ngọc Duy
2015-01-25 12:37 ` [PATCH 16/21] list-files: do not show duplicate cached entries Nguyễn Thái Ngọc Duy
2015-01-25 12:37 ` [PATCH 17/21] list-files: show directories as well as files Nguyễn Thái Ngọc Duy
2015-01-25 19:16   ` Eric Sunshine
2015-01-27 21:51   ` Junio C Hamano [this message]
2015-01-25 12:37 ` [PATCH 18/21] list-files: add -F/--classify Nguyễn Thái Ngọc Duy
2015-01-25 12:37 ` [PATCH 19/21] list-files -F: show submodules with the new indicator '&' Nguyễn Thái Ngọc Duy
2015-01-25 12:37 ` [PATCH 20/21] list-files: -M aka diff-cached Nguyễn Thái Ngọc Duy
2015-01-25 12:37 ` [PATCH 21/21] t3080: tests for git-list-files Nguyễn Thái Ngọc Duy
2015-01-25 19:20   ` Eric Sunshine
2015-01-28  4:44     ` Michael Blume
2015-01-28 10:19       ` Duy Nguyen
2015-01-28 17:49         ` Michael Blume
2015-01-28 19:03       ` Junio C Hamano
  -- strict thread matches above, loose matches on Subject: below --
2015-02-08  9:01 [PATCH 00/21] nd/list-files updates Nguyễn Thái Ngọc Duy
2015-02-08  9:01 ` [PATCH 17/21] list-files: show directories as well as files Nguyễn Thái Ngọc Duy
2015-02-09 22:28   ` Junio C Hamano

find likely ancestor, descendant, or conflicting patches for this message:
( dfblob:29b5c2e dfblob:f28b7e9 )
 OR (
bs:"Re: [PATCH 17/21] list-files: show directories as well as files" )
	(help)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqmw53ki3i.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=pclouds@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

Be sure your reply has a Subject: header at the top and a blank line before the message body.

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