Re: [PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo"

Jeff King [off-list ref] writes:

You can at least lazily do the stat so that only users of foo/ need to
pay the penalty. Something like this (completely untested):

Without "foo/", you do not have to pay the price, so I think
that is a sane optimization, but at the same time it would make
it worse if "foo/" is actually used.  excluded_1() is called for
the same pathname from a loop to check for a match and you would
end up running lstat(2) three times (once each for EXC_CMDL,
EXC_DIRS and EXC_FILE).

But maybe people who want "foo/" deserve it.  I dunno.

In any case, if you do this...

... I think you would need to get the real dtype again in later
part of this function after exclude() decides it should not
ignore it, before the "switch (dtype)" really uses it, on
systems with NO_D_TYPE_IN_DIRENT.

On Thu, Jan 31, 2008 at 02:35:33AM -0800, Junio C Hamano wrote:

Without "foo/", you do not have to pay the price, so I think
that is a sane optimization, but at the same time it would make
it worse if "foo/" is actually used.  excluded_1() is called for
the same pathname from a loop to check for a match and you would
end up running lstat(2) three times (once each for EXC_CMDL,
EXC_DIRS and EXC_FILE).

But maybe people who want "foo/" deserve it.  I dunno.

Ah, I didn't look at it that closely.

To do the laziness right, I think you would need to pass a pointer to
the dtype around, and just fill it in the first time it is needed.

-Peff

Hi,

On Thu, 31 Jan 2008, Jeff King wrote:

On Thu, Jan 31, 2008 at 02:35:33AM -0800, Junio C Hamano wrote:

quoted

Without "foo/", you do not have to pay the price, so I think that is a 
sane optimization, but at the same time it would make it worse if 
"foo/" is actually used.  excluded_1() is called for the same pathname 
from a loop to check for a match and you would end up running lstat(2) 
three times (once each for EXC_CMDL, EXC_DIRS and EXC_FILE).

But maybe people who want "foo/" deserve it.  I dunno.

Ah, I didn't look at it that closely.

To do the laziness right, I think you would need to pass a pointer to 
the dtype around, and just fill it in the first time it is needed.

Just to add my two eurocents: I think the patch is complicated enough that 
we could go the other way round: while parsing the ignore entries, we can 
plainly state that entries with a trailing slash are ignored:

-- snipsnap --
[PATCH] Warn if an ignore/exclude entry ends in a slash

Git does not like ignore entries ending in a slash; they will be ignored.
So just be honest and warn the user about it.

Signed-off-by: Johannes Schindelin <redacted>

---

 dir.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/dir.c b/dir.c
index 1b9cc7a..c3e9a0d 100644
--- a/dir.c
+++ b/dir.c

@@ -135,6 +135,11 @@ void add_exclude(const char *string, const char *base,
 	}
 	x->pattern = string;
 	x->patternlen = strlen(string);
+	if (x->patternlen && x->pattern[x->patternlen - 1] == '/') {
+		warning("Ignoring ignore entry because of trailing slash: %s",
+			string);
+		return;
+	}
 	x->base = base;
 	x->baselen = baselen;
 	x->flags = 0;

On 31/01/2008, Johannes Schindelin [off-list ref] wrote:
[snip]

How about something like,
                  warning("Ignoring ignore entry because of trailing
slash: %s\n Remove the trailing slash from the directory name to
ignore it", string);

May be this will help absolute git newbies.
Please ignore this if it sounds like a "too trivial, everyone should
know this" case.

Thanks,
           --Pradeep

+               return;
+       }
        x->base = base;
        x->baselen = baselen;
        x->flags = 0;

-- 
Pradeep Singh Rautela
http://eagain.wordpress.com
http://emptydomain.googlepages.com

* Johannes Schindelin [31 I 2008 12:38]:

Just to add my two eurocents: I think the patch is complicated enough that 
we could go the other way round: while parsing the ignore entries, we can 
plainly state that entries with a trailing slash are ignored:

-- snipsnap --
[PATCH] Warn if an ignore/exclude entry ends in a slash

Git does not like ignore entries ending in a slash; they will be ignored.
So just be honest and warn the user about it.

Signed-off-by: Johannes Schindelin <redacted>

I agree that this is a reasonable remedy for this issue. So:

Acked-by: Adam Piątyszek <redacted>

BTW, the warning message is a bit "hidden" between the "Changed" and 
"Untracked" parts of a status message, e.g.:

===== >8 =====
# On branch master
# Changed but not updated:
#   (use "git add <file>..." to update what will be committed)
#
#       modified:   src/Makefile
#       modified:   src/ofdm.cpp
#
warning: Ignoring ignore entry because of trailing slash: results/
# Untracked files:
#   (use "git add <file>..." to include in what will be committed)
#
#       results/
no changes added to commit (use "git add" and/or "git commit -a")
===== >8 =====

Is it possible to make warnings displayed in red or yellow colour on 
terminals that support colours?

BR,
/Adam


-- 
.:.  Adam Piatyszek (ediap)  .:.....................................:.
.:.  ediap@users.sourceforge.net  .:................................:.