[PATCH 1/1] Tell vim the textwidth is 75.

[PATCH 1/1] Tell vim the textwidth is 75.

[Bryan larsen]

When invoking EDITOR, add some metadata to tell vim the textwidth is 75.

Signed-off-by: Bryan Larsen <bryan.larsen@gmail.com>
---

 stgit/stack.py |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/stgit/stack.py b/stgit/stack.py
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -66,6 +66,8 @@ def edit_file(string, comment):
           % __comment_prefix
     print >> f, __comment_prefix, \
           'Trailing empty lines will be automatically removed.'
+    print >> f, __comment_prefix, \
+          'vim: textwidth=75'
     f.close()
 
     # the editor

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Junio C Hamano]

I do not do Porcelain, but wouldn't it be nicer if we had a
Porcelain neutral "commit log template file" under $GIT_DIR
somewhere?  'vim: textwidth=75' is completely useless for
somebody like me (I almost always work inside Emacs).

Cogito seems to use $GIT_DIR/commit-template for that purpose.
Can't users put that "vim:" hint there, and if StGIT does not
use a commit template, patch it to use the same file as Cogito
does?

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Catalin Marinas]

Junio C Hamano <junkio@cox.net> wrote:
> I do not do Porcelain, but wouldn't it be nicer if we had a
> Porcelain neutral "commit log template file" under $GIT_DIR
> somewhere?  'vim: textwidth=75' is completely useless for
> somebody like me (I almost always work inside Emacs).

StGIT uses .git/patchdescr.tmpl as the template, where people can put
the a line like "STG: vim: textwidth=75" which will be automatically
removed. I won't include this patch since it is up to the user to
define whatever setting he/she wants for his editor (I use emacs
myself and add something like "STG: -*- mode: text; -*-" on the first
line)

> Cogito seems to use $GIT_DIR/commit-template for that purpose.
> Can't users put that "vim:" hint there, and if StGIT does not
> use a commit template, patch it to use the same file as Cogito
> does?

I would use a neutral commit template, only that it should have a
neutral prefix as well for the lines to be removed (neither STG nor CG
but GIT maybe). The $GIT_DIR/commit-template is fine as a file name.

-- 
Catalin

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Sam Ravnborg]

> 
> I would use a neutral commit template, only that it should have a
> neutral prefix as well for the lines to be removed (neither STG nor CG
> but GIT maybe). The $GIT_DIR/commit-template is fine as a file name.

How about $GIT_DIR/commit-template-`basename $EDITOR`
Then we could have different templates for vim, emacs, kade etc.

	Sam

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Junio C Hamano]

Sam Ravnborg <sam@ravnborg.org> writes:

>> I would use a neutral commit template, only that it should have a
>> neutral prefix as well for the lines to be removed (neither STG nor CG
>> but GIT maybe). The $GIT_DIR/commit-template is fine as a file name.
>
> How about $GIT_DIR/commit-template-`basename $EDITOR`
> Then we could have different templates for vim, emacs, kade etc.

This brings up a point I have been wanting to see discussed,
involving the core people and the Porcelain people [*1*].

I would like to see Porcelains stay compatible when the do not
have to differ.  The commit template [*2*] is one example of
such.  Another example is the "dontdiff/ignore" file Pasky
talked about in a recent commit log in his Cogito tree [*3*].

Porcelains need to agree on what is placed where and used in
what way.

First, I will talk about the "what" part.  I can see there are
various "preference" items we may want to use:

  - commit template (to enforce a certain style)
  - standard "dontdiff/ignore" file.
  - pre-commit hook (to enforce a certain tests to pass)
  - post-commit-hook (sending commit-notification perhaps).
  - environment overrides (COMMITTER_NAME, COMMITTER_EMAIL and
    such).

There may be others.  Many of them would have different origin:

  - Per project.  A project may want to enforce pre-commit hook
    for all participants;

  - Per user.  A user may want to use different environment
    settings for different projects [*4*].

  - Per repository (or work tree).  A user may have more than
    one work tree for the same project, and want to use
    different "preference" items per tree.

Personally, given the nature of GIT being a distributed system,
I do not think something like /etc/git.conf (which suggests "per
system" configuration) makes much sense; except working around a
mailhost name configuration, perhaps.

About the "where" part, one proposal I have off the top of my
head is something like this:

  - Have a directory at the root of the tree, "_git" (I do not
    care about the name at this moment.  The point being it can
    be revision controlled as part of the project and propagate
    to other repositories), to store per-project configuration.

  - Use $GIT_DIR/conf/ as a convention to store per repository
    configuration files.  This does not propagate with
    pulls/pushes/merges across repositories.

  - Use $HOME/.gitrc (could be a directory or a file in .ini
    style like StGIT uses -- again, I do not care about the
    details at this moment) to store per-user configuration.

Which configuration is read first, what can be overridden, and
if the configuration is cumulative would be specific to each
preference item, I suspect.  Some project may not want a user to
override the pre-commit hooks, for a bad example.  But normally
the per-repository one would take precedence over per-user one
which in turn would take precedence over per-project one.


[Footnotes]

*1* Technically this does not involve the core at all, but the
core people can act as objective, Porcelain-neutral referees.
They'll need to know the outcome of the discussion anyway, since
they are the ones that end up maintaining the Porcelain-neutral
tutorial document.

*2* Unless we are talking about the kind that shows and lets you
edit the diff to be committed, which somebody else's Porcelain
may support, that is.

*3* .gitignore in the cwd is used in Cogito, if I am not
mistaken.

*4* E.g. I would commit for GIT project with junkio@cox.net
while using junio@twinsun.com for my day-job projects.

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Petr Baudis]

Dear diary, on Fri, Jul 22, 2005 at 12:37:05PM CEST, I got a letter
where Catalin Marinas <catalin.marinas@gmail.com> told me that...
> > Cogito seems to use $GIT_DIR/commit-template for that purpose.
> > Can't users put that "vim:" hint there, and if StGIT does not
> > use a commit template, patch it to use the same file as Cogito
> > does?
> 
> I would use a neutral commit template, only that it should have a
> neutral prefix as well for the lines to be removed (neither STG nor CG
> but GIT maybe). The $GIT_DIR/commit-template is fine as a file name.

This unfortunately isn't that simple, since this file substitutes only
the

	CG: -----------------------------------------------------------------------
	CG: Lines beginning with the CG: prefix are removed automatically.

snippet of the file. The trouble is, Cogito autogenerates most of the
rest of it. Would the acceptable solution be that I would have
@CG_FILELIST@-style placeholders there, and any tool processing the
file would simply drop lines containing @ directives it does not
understand? (@@ is escaped @)

I have nothing against changing the prefix to GIT:.

Then, Cogito's default commit-template would look like

	GIT: -----------------------------------------------------------------------
	GIT: Lines beginning with the GIT: prefix are removed automatically.
	GIT:
	GIT: Author: @AUTHOR_NAME@
	GIT: Email: @AUTHOR_EMAIL@
	GIT: Date: @AUTHOR_DATE@
	GIT:@CG_SHOWFILES@@CG_NOMERGE@
	GIT:@CG_SHOWFILES@@CG_NOMERGE@ By deleting lines beginning with GIT:F, the associated file
	GIT:@CG_SHOWFILES@@CG_NOMERGE@ will be removed from the commit list.
	GIT:@CG_SHOWFILES@
	GIT:@CG_SHOWFILES@ Modified files:
	GIT:@CG_SHOWFILES@F   @FILELIST@
	GIT: -----------------------------------------------------------------------
	GIT: vim: textwidth=75

(where CG_SHOWFILES is defined only when the list of the files is to be
shown and CG_NOMERGE only when there is no merge going on).

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
If you want the holes in your knowledge showing up try teaching
someone.  -- Alan Cox

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Petr Baudis]

Dear diary, on Fri, Jul 22, 2005 at 10:39:06PM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> told me that...
> Porcelains need to agree on what is placed where and used in
> what way.

Yes, I always try to make things as Cogito-unspecific as possible.

>   - Per user.  A user may want to use different environment
>     settings for different projects [*4*].
> 
>   - Per repository (or work tree).  A user may have more than
>     one work tree for the same project, and want to use
>     different "preference" items per tree.

I wouldn't distinct between those two. You as a user can copy things
around when you clone the repositories (if you ever do so), or the
Porcelain can do it, but having global per-user _combined with
per-project settings sounds nightmarish. After all, how do you identify
a project?  What if projects merge? I wouldn't open this can of worms.

Obviously, there have to be just per-user settings (independent of
"project").

> About the "where" part, one proposal I have off the top of my
> head is something like this:
> 
>   - Have a directory at the root of the tree, "_git" (I do not
>     care about the name at this moment.  The point being it can
>     be revision controlled as part of the project and propagate
>     to other repositories), to store per-project configuration.

Fine, but I would probably prefer having it hidden. .gitinfo?

>   - Use $GIT_DIR/conf/ as a convention to store per repository
>     configuration files.  This does not propagate with
>     pulls/pushes/merges across repositories.

That's fine by me. I'd prefer the hooks staying in $GIT_DIR/hooks/.

>   - Use $HOME/.gitrc (could be a directory or a file in .ini
>     style like StGIT uses -- again, I do not care about the
>     details at this moment) to store per-user configuration.

As long as I can sanely parse it in shell... ;-)

> Which configuration is read first, what can be overridden, and
> if the configuration is cumulative would be specific to each
> preference item, I suspect.  Some project may not want a user to
> override the pre-commit hooks, for a bad example.  But normally
> the per-repository one would take precedence over per-user one
> which in turn would take precedence over per-project one.

What about just running all the hooks in the order you specified?

> [Footnotes]
> 
> *1* Technically this does not involve the core at all, but the
> core people can act as objective, Porcelain-neutral referees.
> They'll need to know the outcome of the discussion anyway, since
> they are the ones that end up maintaining the Porcelain-neutral
> tutorial document.
> 
> *2* Unless we are talking about the kind that shows and lets you
> edit the diff to be committed, which somebody else's Porcelain
> may support, that is.

FWIW, I'm planning something like this in cg-commit (when called
explicitly with -d) in short-term future.

> *3* .gitignore in the cwd is used in Cogito, if I am not
> mistaken.

Yes. There were several discussions about this in the past, with no
clear outcome, IIRC. I would prefer:

  ~/.git/ignore per-user
  /.git/ignore per-repository
  .gitignore per-directory (cummulative with parent directories)

Note that I also want to make use of some special characters in this
file. In particular /^# and /^!, to make it at least as powerful as CVS'
ignore.

> *4* E.g. I would commit for GIT project with junkio@cox.net
> while using junio@twinsun.com for my day-job projects.

.git/author in current Cogito (.git/conf/author or something in the
Brave New World ;-).

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
If you want the holes in your knowledge showing up try teaching
someone.  -- Alan Cox

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Catalin Marinas]

On Fri, 2005-07-22 at 19:24 +0000, Sam Ravnborg wrote:
> > I would use a neutral commit template, only that it should have a
> > neutral prefix as well for the lines to be removed (neither STG nor CG
> > but GIT maybe). The $GIT_DIR/commit-template is fine as a file name.
> 
> How about $GIT_DIR/commit-template-`basename $EDITOR`
> Then we could have different templates for vim, emacs, kade etc.

I'm not sure this is worth the hassle since a person usually sticks with
one editor, I don't see why one would use different $EDITOR variables
with the same project.

-- 
Catalin

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Junio C Hamano]

Wonderful start.

Later on, Porcelains could agree on what @TOKEN@ are generally
available, and even start using a common script to pre-fill the
templates, like:

  $ git-fill-template-script <template> <output-file> var=val var=val...

In your example, I see AUTHOR_NAME, AUTHOR_EMAIL, and
AUTHOR_DATE (I'd use GIT_AUTHOR_NAME etc to match existing
environment variables, though) would be something that are
probably common across Porcelains, and the Porcelain would not
even have to bother passing them as the command argument to
fill-template.  About FILELIST, the default would be to do
"git-diff-cache --name-only HEAD", but if a Porcelain keeps
track of "modified" files differently it can be overridden by
passing FILELIST as an explicit parameter.

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Petr Baudis]

Dear diary, on Fri, Jul 22, 2005 at 11:16:51PM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> told me that...
> Wonderful start.
> 
> Later on, Porcelains could agree on what @TOKEN@ are generally
> available, and even start using a common script to pre-fill the
> templates, like:
> 
>   $ git-fill-template-script <template> <output-file> var=val var=val...
> 
> In your example, I see AUTHOR_NAME, AUTHOR_EMAIL, and
> AUTHOR_DATE (I'd use GIT_AUTHOR_NAME etc to match existing
> environment variables, though) would be something that are
> probably common across Porcelains, and the Porcelain would not
> even have to bother passing them as the command argument to
> fill-template.

Good idea. More interesting exercise would be to make a script which
extracts the values back after the user had a chance to touch it.

> About FILELIST, the default would be to do "git-diff-cache --name-only
> HEAD", but if a Porcelain keeps track of "modified" files differently
> it can be overridden by passing FILELIST as an explicit parameter.

Cogito shows '[NMD] filename' in place of @FILELIST@.

This brings me to another subject, M and N are pretty hard to
distinguish visually without close inspection of the output. What about
switching to use A instead of N everywhere?

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
If you want the holes in your knowledge showing up try teaching
someone.  -- Alan Cox

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Catalin Marinas]

On Fri, 2005-07-22 at 13:39 -0700, Junio C Hamano wrote:
> I would like to see Porcelains stay compatible when the do not
> have to differ.  The commit template [*2*] is one example of
> such.  

For StGIT it is not a problem to use any commit template with any
prefix. It doesn't generate extra lines.

Would such a template only have 'GIT:' prefixed lines? I usually put
another line like 'Signed-off-by:', for convenience. The problem with
StGIT appears when one wants to re-edit the patch description (stg
refresh -e), in which case the existing description should be merged
with a part of the template (if you want to get the editor setting for
example). It doesn't do this since there is no point in getting another
'Signed...' line in the existing description.

> First, I will talk about the "what" part.  I can see there are
> various "preference" items we may want to use:
> 
>   - commit template (to enforce a certain style)

OK

>   - standard "dontdiff/ignore" file.

StGIT currently uses .git/exclude, since I saw it used by cogito. What
is dontdiff supposed to do? The 'git diff' command only shows the diff
for the files added to the repository.

>   - pre-commit hook (to enforce a certain tests to pass)
>   - post-commit-hook (sending commit-notification perhaps).

OK

>   - environment overrides (COMMITTER_NAME, COMMITTER_EMAIL and
>     such).

StGIT works the other way around. By default uses the environment, which
can be overridden by the stgitrc file. I could change this easily.

> There may be others.  Many of them would have different origin:
> 
>   - Per project.  A project may want to enforce pre-commit hook
>     for all participants;

As Petr said, it's hard to define a project.

>   - Per user.  A user may want to use different environment
>     settings for different projects [*4*].
> 
>   - Per repository (or work tree).  A user may have more than
>     one work tree for the same project, and want to use
>     different "preference" items per tree.

StGIT uses /etc/stgitrc, ~/.stgitrc and .git/stgitrc, the latter
overriding the former.

> Personally, given the nature of GIT being a distributed system,
> I do not think something like /etc/git.conf (which suggests "per
> system" configuration) makes much sense; except working around a
> mailhost name configuration, perhaps.

For StGIT it makes sense to get some default settings via /etc/stgitrc.
There are things like a SMTP server and the diff3 command. These are set
when installing the application and can be overridden in your home
or .git directories.

> About the "where" part, one proposal I have off the top of my
> head is something like this:

Before we get to "where", we should define the common settings. I think
that git should define the common settings for its operations and the
other tools should follow them.

Once you get unique settings for an application (like mail templates or
three-way merge commands), it's pretty hard to put them in the same
file. It would even be confusing for users.

>   - Have a directory at the root of the tree, "_git" (I do not
>     care about the name at this moment.  The point being it can
>     be revision controlled as part of the project and propagate
>     to other repositories), to store per-project configuration.

That's the thing I didn't like in GNU Arch. You modify the file ignoring
rules for example and the change will be included in the next commit.
You could only get some defaults when cloning a repository, otherwise
once you have different preferences from the repository's maintainer,
you start getting conflicts in the config files.

>   - Use $GIT_DIR/conf/ as a convention to store per repository
>     configuration files.  This does not propagate with
>     pulls/pushes/merges across repositories.

That's fine.

>   - Use $HOME/.gitrc (could be a directory or a file in .ini
>     style like StGIT uses -- again, I do not care about the
>     details at this moment) to store per-user configuration.

Again, having Porcelain specific options mixed in the same file might
lead to some confusion among users.

> But normally
> the per-repository one would take precedence over per-user one
> which in turn would take precedence over per-project one.

With a note if specifying what a project is.

> *3* .gitignore in the cwd is used in Cogito, if I am not
> mistaken.

I will to add this to StGIT.

> *4* E.g. I would commit for GIT project with junkio@cox.net
> while using junio@twinsun.com for my day-job projects.

In StGIT this is settable via authorname/authoremail in the stgitrc file
and can be per repository or per user.

-- 
Catalin

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Junio C Hamano]

Catalin Marinas <catalin.marinas@gmail.com> writes:

> Would such a template only have 'GIT:' prefixed lines? I usually put
> another line like 'Signed-off-by:', for convenience. The problem with
> StGIT appears when one wants to re-edit the patch description (stg
> refresh -e), in which case the existing description should be merged
> with a part of the template (if you want to get the editor setting for
> example). It doesn't do this since there is no point in getting another
> 'Signed...' line in the existing description.

If signed-off-by is the only thing you are worried about, how
about making it not part of the commit template and the message
user touches with the editor?  You first look at the user
configuration somewhere to see if the user wants the
signed-off-by line to his commits and with what value, and if
the last lines of the edit result does not contain that value
(to avoid duplicates), add it before feeding the message to
git-commit-tree.

>>   - standard "dontdiff/ignore" file.
>
> StGIT currently uses .git/exclude, since I saw it used by cogito. What
> is dontdiff supposed to do? The 'git diff' command only shows the diff
> for the files added to the repository.

I see that what I wrote was vague and badly stated.  Please
forget about my mentioning "dontdiff".  What I meant was your
.git/exclude, Pasky's .gitignore file and friends.

>>   - environment overrides (COMMITTER_NAME, COMMITTER_EMAIL and
>>     such).
>
> StGIT works the other way around. By default uses the environment, which
> can be overridden by the stgitrc file. I could change this easily.

Again I was vague, and what you say StGIT does is exactly what I
meant.  I have one value in my environment coming from the login
shell, and a per- repository preference item overrides it to
something else.

>>   - Per project.  A project may want to enforce pre-commit hook
>>     for all participants;
>
> As Petr said, it's hard to define a project.

By reading the part you talk about your hating tla, I think you
know exactly what I mean.

When you merge two projects like Linus did between git.git and
gitk, obviously the person who is merging the two is responsible
for merging the per-project default configuration and resolving
conflicts.  This probably should be overridable by individual
developers who pull/fetch into their repository by having per-
repository configuration.

> For StGIT it makes sense to get some default settings via /etc/stgitrc.
> There are things like a SMTP server and the diff3 command. These are set
> when installing the application and can be overridden in your home
> or .git directories.

Exactly, but that is not specific to StGIT, I presume, and I did
not want to hear "``For StGIT'' it makes sense".  If StGIT needs
to use "diff3" on a system, probably that is because "merge" is
not available on that system.  In that case,  cogito needs to
use it too, doesn't it?

If we can make users and sysadmins not having to maintain two
sets of configuration files for two Porcelains, if we
can,... that is what I have been trying to address.

> Before we get to "where", we should define the common
> settings. I think that git should define the common settings
> for its operations and the other tools should follow them.

Personally, unless it is something very obvious and basic, I do
not think the core barebone Porcelain should be inventing
arbitrary conventions and imposing them on other Porcelains.
For very basic things I would agree.

I think Petr already started the discussion rolling for commit
templates, and I like his proposal.  For ignore pattern files, I
think what Cogito does sounds almost sensible [*1*] and I am
sure StGIT have something similar.  I do not see Linus and co
jumping up and down saying git-status should detect and show new
files not registered in the cache, so for now I'd propose to
skip adding this one to the barebone Porcelain (meaning, this is
an example of not "git defining the common and others following
suite").

> That's the thing I didn't like in GNU Arch. You modify the file ignoring
> rules for example and the change will be included in the next commit.
> You could only get some defaults when cloning a repository, otherwise
> once you have different preferences from the repository's maintainer,
> you start getting conflicts in the config files.

That's why I suggested to have "_git" (project wide default)
separate from $GIT_DIR/info (repository owner's discretion), the
latter overriding the former.

>>   - Use $HOME/.gitrc (could be a directory or a file in .ini
>>     style like StGIT uses -- again, I do not care about the
>>     details at this moment) to store per-user configuration.
>
> Again, having Porcelain specific options mixed in the same file might
> lead to some confusion among users.

True.  We need to be careful.

Or course, there is an option of not worry about Porcelain
compatibilities at all --- which is certainly simpler.  All we
need is to make sure they do not use the same filename for
conflicting purposes.  If everybody feels that way then this
discussion is moot and I apologize for wasting people's time.

[Footnote]

*1* I said "almost sensible" but it is not meant to blame Pasky.
I think the --exclude mechanism in git-ls-files should be
extended to allow not just the filename-sans-leading-directory
match but a full relative-to-the-project-root path match.  That
way, cg-status would not have to run around in the tree to find
individual .gitignore files.  

Personally, I think having to have ignore pattern like .cvsignore
per-directory is simply _ugly_.

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> Cogito shows '[NMD] filename' in place of @FILELIST@.

Sounds sensible.  Does it parse it to limit the files to be
committed?

> This brings me to another subject, M and N are pretty hard to
> distinguish visually without close inspection of the output. What about
> switching to use A instead of N everywhere?

Although I admit that is minor, I've had the same problem, and
this sounds like a good idea.

However, I'd like to see what the extent of damage would be even
if everybody agrees this is a good change.  Any one of core
barebone Porcelain, Linus git-tools, gitk, gitweb, Cogito, and
StGIT would have a veto over this kind of change, or at least we
should wait until everybody catches up.

If we all decide to go ahead, the transition would not be so
painful, since we do not currently say 'A', the Porcelains can
start accepting 'A' as synonym for 'N' today, and then later we
can change the tools to produce 'A' instead of 'N'.

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Petr Baudis]

Dear diary, on Sat, Jul 23, 2005 at 01:24:35AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> told me that...
> Petr Baudis <pasky@suse.cz> writes:
> 
> > Cogito shows '[NMD] filename' in place of @FILELIST@.
> 
> Sounds sensible.  Does it parse it to limit the files to be
> committed?

Yes.

> > This brings me to another subject, M and N are pretty hard to
> > distinguish visually without close inspection of the output. What about
> > switching to use A instead of N everywhere?
> 
> Although I admit that is minor, I've had the same problem, and
> this sounds like a good idea.
> 
> However, I'd like to see what the extent of damage would be even
> if everybody agrees this is a good change.  Any one of core
> barebone Porcelain, Linus git-tools, gitk, gitweb, Cogito, and
> StGIT would have a veto over this kind of change, or at least we
> should wait until everybody catches up.

I don't think the situation is so bad. At least in Cogito, there is only
one use of 'N' (and cg-status, but that goes directly to the user), and
it's likely useless and things would work even with that changing to 'A'
(nevertheless, I just updated Cogito to accept 'A' at that place as
well).

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
If you want the holes in your knowledge showing up try teaching
someone.  -- Alan Cox

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Catalin Marinas]

On Fri, 2005-07-22 at 16:07 -0700, Junio C Hamano wrote:
> Catalin Marinas <catalin.marinas@gmail.com> writes:
>
> If signed-off-by is the only thing you are worried about, how
> about making it not part of the commit template and the message
> user touches with the editor?  You first look at the user
> configuration somewhere to see if the user wants the
> signed-off-by line to his commits and with what value, and if
> the last lines of the edit result does not contain that value
> (to avoid duplicates), add it before feeding the message to
> git-commit-tree.

That's an idea.

Another problem with the template is when one wants a header as well as
footer (for things like '-*- mode: text; -*-'). Maybe something like
below would work:

GIT: your header
@DESCRIPTION@
GIT: your footer
GIT: @FILELIST@

where @DESCRIPTION@ is either a blank line for cogito or the existing
patch description for StGIT. One could also add a 'Signed-...' line when
the patch is first created (instead of a blank line).

For StGIT, one could add something like @PATCHNAME@ as well.

> > StGIT currently uses .git/exclude, since I saw it used by cogito. What
> > is dontdiff supposed to do? The 'git diff' command only shows the diff
> > for the files added to the repository.
> 
> I see that what I wrote was vague and badly stated.  Please
> forget about my mentioning "dontdiff".  What I meant was your
> .git/exclude, Pasky's .gitignore file and friends.

.gitignore are note currently supported by StGIT but I'll add this.

> When you merge two projects like Linus did between git.git and
> gitk, obviously the person who is merging the two is responsible
> for merging the per-project default configuration and resolving
> conflicts.  This probably should be overridable by individual
> developers who pull/fetch into their repository by having per-
> repository configuration.

The problem appears when one upstream maintainer changes the
configuration, should this be merged again? In this case you can get
conflicts.

> > For StGIT it makes sense to get some default settings via /etc/stgitrc.
> > There are things like a SMTP server and the diff3 command. These are set
> > when installing the application and can be overridden in your home
> > or .git directories.
> 
> Exactly, but that is not specific to StGIT, I presume, and I did
> not want to hear "``For StGIT'' it makes sense".  If StGIT needs
> to use "diff3" on a system, probably that is because "merge" is
> not available on that system.  In that case,  cogito needs to
> use it too, doesn't it?

This is not always the case. With StGIT you can define your own options
and tools for a three-way merge. This was implemented because Bryan
Larsen, I think, asked whether a different (smarter) tool could be used.
One might also want that when diff3 fails, a xxdiff or emacs should be
automatically started for the conflict files.

This could be simplified if we enforce the presence of a gitmerge.sh
file which only calls merge or diff3 by default. Users can create a new
file and put it in the $PATH.

> If we can make users and sysadmins not having to maintain two
> sets of configuration files for two Porcelains, if we
> can,... that is what I have been trying to address.

That's probably a good reason. Also people might use 2 Porcelains and
the plain git, they could have a common configuration, especially where
settings overlap.

> I think Petr already started the discussion rolling for commit
> templates, and I like his proposal.

I like it too.

> > That's the thing I didn't like in GNU Arch. You modify the file ignoring
> > rules for example and the change will be included in the next commit.
> > You could only get some defaults when cloning a repository, otherwise
> > once you have different preferences from the repository's maintainer,
> > you start getting conflicts in the config files.
> 
> That's why I suggested to have "_git" (project wide default)
> separate from $GIT_DIR/info (repository owner's discretion), the
> latter overriding the former.

That's OK with one issue - git should be able to exclude _git when
generating a diff between 2 trees, unless one can enforce the _git/*
files to be read-only.

Another option would be to have .git/info/<branch> and, with cogito for
example, .git/info/origin should always be pulled, even if the local
files were modified. You would override these settings
in .git/info/master. The problem is to define the branches order in
which the settings are read.

> > Again, having Porcelain specific options mixed in the same file might
> > lead to some confusion among users.
> 
> True.  We need to be careful.

This could be avoided by using ini-like files (well, easy to read in
Python) and have [git] (for the common things like author name),
[cogito], [stgit] etc. sections.

> Or course, there is an option of not worry about Porcelain
> compatibilities at all --- which is certainly simpler.  All we
> need is to make sure they do not use the same filename for
> conflicting purposes.  If everybody feels that way then this
> discussion is moot and I apologize for wasting people's time.

I don't think this is a waste of time. It's useful to have at least some
basic conventions. StGIT places files all over the place but without any
convention, not even a .stgit extension.

The problem is how much similar we want the Porcelains to be regarding
the settings and the templates. For StGIT, it is much simpler to have
something like '%(FILELIST)s' rather than '@FILELIST@' in a template but
I have not problem with switching to a common syntax. But we should see
what can easily be changed.

I will write a list with what files StGIT uses and where they are placed
and we can agree on a structure. I think the .git/ directory usage is
more important to be clarified than having a common {git,cogito,stgit}rc
file.

-- 
Catalin

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Petr Baudis]

Dear diary, on Sat, Jul 23, 2005 at 01:07:05AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> told me that...
> Catalin Marinas <catalin.marinas@gmail.com> writes:
> 
> > Would such a template only have 'GIT:' prefixed lines? I usually put
> > another line like 'Signed-off-by:', for convenience. The problem with
> > StGIT appears when one wants to re-edit the patch description (stg
> > refresh -e), in which case the existing description should be merged
> > with a part of the template (if you want to get the editor setting for
> > example). It doesn't do this since there is no point in getting another
> > 'Signed...' line in the existing description.
> 
> If signed-off-by is the only thing you are worried about, how
> about making it not part of the commit template and the message
> user touches with the editor?  You first look at the user
> configuration somewhere to see if the user wants the
> signed-off-by line to his commits and with what value, and if
> the last lines of the edit result does not contain that value
> (to avoid duplicates), add it before feeding the message to
> git-commit-tree.

I would rather have something universal. Just avoid inserting duplicate
lines at the bottom.

> >>   - standard "dontdiff/ignore" file.
> >
> > StGIT currently uses .git/exclude, since I saw it used by cogito. What
> > is dontdiff supposed to do? The 'git diff' command only shows the diff
> > for the files added to the repository.
> 
> I see that what I wrote was vague and badly stated.  Please
> forget about my mentioning "dontdiff".  What I meant was your
> .git/exclude, Pasky's .gitignore file and friends.

Cogito supports .git/exclude too, but I'd rather rename it to
.git/ignore (or .git/conf/ignore) as well (gradually).

> >>   - environment overrides (COMMITTER_NAME, COMMITTER_EMAIL and
> >>     such).
> >
> > StGIT works the other way around. By default uses the environment, which
> > can be overridden by the stgitrc file. I could change this easily.
> 
> Again I was vague, and what you say StGIT does is exactly what I
> meant.  I have one value in my environment coming from the login
> shell, and a per- repository preference item overrides it to
> something else.

I have it the other way around, with the rationale that your default
settings should be in your ~/.gitrc, not environment, which is always
the highest priority. The simple reason is that how would I apply the
patches of other people otherwise? Now I do

	GIT_AUTHOR_NAME="Junio C Hamano" \
		GIT_AUTHOR_EMAIL="junkio@cox.net" \
		GIT_AUTHOR_DATE="2008-04-01 05:12:33" \
		cg-commit

and it makes complete sense to me.

> > For StGIT it makes sense to get some default settings via /etc/stgitrc.
> > There are things like a SMTP server and the diff3 command. These are set
> > when installing the application and can be overridden in your home
> > or .git directories.
> 
> Exactly, but that is not specific to StGIT, I presume, and I did
> not want to hear "``For StGIT'' it makes sense".  If StGIT needs
> to use "diff3" on a system, probably that is because "merge" is
> not available on that system.  In that case,  cogito needs to
> use it too, doesn't it?

diff3 throws its output on stdout, AFAIK.

> If we can make users and sysadmins not having to maintain two
> sets of configuration files for two Porcelains, if we
> can,... that is what I have been trying to address.

Yes, but it is a bit secondary to me. The most important for me is that
meta-files inside the project itself (e.g. .gitignore) should be as
portable as possible, as that'd be the biggest hurdle. The second
priority for me is to make ~/.git/ as universal as possible. Having
common per-user/per-system configuration file as well is nice too, but
not so crucial.

> > Before we get to "where", we should define the common
> > settings. I think that git should define the common settings
> > for its operations and the other tools should follow them.
> 
> Personally, unless it is something very obvious and basic, I do
> not think the core barebone Porcelain should be inventing
> arbitrary conventions and imposing them on other Porcelains.
> For very basic things I would agree.
> 
> I think Petr already started the discussion rolling for commit
> templates, and I like his proposal.  For ignore pattern files, I
> think what Cogito does sounds almost sensible [*1*] and I am
> sure StGIT have something similar.  I do not see Linus and co
> jumping up and down saying git-status should detect and show new
> files not registered in the cache, so for now I'd propose to
> skip adding this one to the barebone Porcelain (meaning, this is
> an example of not "git defining the common and others following
> suite").

(Quite some things came to git from Cogito anyway. ;-) And well, that's
completely natural.)

> *1* I said "almost sensible" but it is not meant to blame Pasky.
> I think the --exclude mechanism in git-ls-files should be
> extended to allow not just the filename-sans-leading-directory
> match but a full relative-to-the-project-root path match.  That
> way, cg-status would not have to run around in the tree to find
> individual .gitignore files.  
> 
> Personally, I think having to have ignore pattern like .cvsignore
> per-directory is simply _ugly_.

No, I think it's great. That increases the locality of things, which is
good. Think about it as of variables - it's nicer to have them local.
Also, with a central .gitignore file, how do you specify "Ignore all
*.html files under Documentation/" or so? (Without having ** - or you
need to support that one too to make the central .gitignore feasible.
But I think that in that case, it would depend on the user whether he
defines things in the central .gitignore or in subdirectories.)

> >>   - Use $HOME/.gitrc (could be a directory or a file in .ini
> >>     style like StGIT uses -- again, I do not care about the
> >>     details at this moment) to store per-user configuration.
> >
> > Again, having Porcelain specific options mixed in the same file might
> > lead to some confusion among users.
> 
> True.  We need to be careful.
> 
> Or course, there is an option of not worry about Porcelain
> compatibilities at all --- which is certainly simpler.  All we
> need is to make sure they do not use the same filename for
> conflicting purposes.  If everybody feels that way then this
> discussion is moot and I apologize for wasting people's time.

I don't think it is moot at all. (But see above - if we can't agree on
everything, I would much rather have common stuff in the project tree
than in ~ - there's not so much trouble of having multiple sets in ~,
but it's annoying to have e.g. .stgitignore, .cgignore and .jitignore in
each directory of the project, and if one project developer moves to
another Porcelain, having to add another set of files, and then keeping
them all up to date...)

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
If you want the holes in your knowledge showing up try teaching
someone.  -- Alan Cox

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Petr Baudis]

Dear diary, on Sat, Jul 23, 2005 at 10:41:38AM CEST, I got a letter
where Catalin Marinas <catalin.marinas@gmail.com> told me that...
> Another problem with the template is when one wants a header as well as
> footer (for things like '-*- mode: text; -*-'). Maybe something like
> below would work:
> 
> GIT: your header
> @DESCRIPTION@
> GIT: your footer
> GIT: @FILELIST@
> 
> where @DESCRIPTION@ is either a blank line for cogito or the existing
> patch description for StGIT. One could also add a 'Signed-...' line when
> the patch is first created (instead of a blank line).
> 
> For StGIT, one could add something like @PATCHNAME@ as well.

Great idea.

> > When you merge two projects like Linus did between git.git and
> > gitk, obviously the person who is merging the two is responsible
> > for merging the per-project default configuration and resolving
> > conflicts.  This probably should be overridable by individual
> > developers who pull/fetch into their repository by having per-
> > repository configuration.
> 
> The problem appears when one upstream maintainer changes the
> configuration, should this be merged again? In this case you can get
> conflicts.

So you resolve them...? If the upstream keeps doing changes frequent
enough and large-scale enough to this becoming annoying, something is
wrong. :-)

> > > That's the thing I didn't like in GNU Arch. You modify the file ignoring
> > > rules for example and the change will be included in the next commit.
> > > You could only get some defaults when cloning a repository, otherwise
> > > once you have different preferences from the repository's maintainer,
> > > you start getting conflicts in the config files.
> > 
> > That's why I suggested to have "_git" (project wide default)
> > separate from $GIT_DIR/info (repository owner's discretion), the
> > latter overriding the former.
> 
> That's OK with one issue - git should be able to exclude _git when
> generating a diff between 2 trees, unless one can enforce the _git/*
> files to be read-only.

Why? I think those meta-information is important too, and if it differs,
I want to see it in the diff. Oh, now I see what you mean - to
optionally exclude it. That would be nice, having --exclude in
common diff options.

> Another option would be to have .git/info/<branch> and, with cogito for
> example, .git/info/origin should always be pulled, even if the local
> files were modified. You would override these settings
> in .git/info/master. The problem is to define the branches order in
> which the settings are read.

Yes, and you may be pulling from multiple branches. I would keep
.git/info simple and single-instanced. If you want your stuff to
propagate to others, put it to .gitinfo/.

> > > Again, having Porcelain specific options mixed in the same file might
> > > lead to some confusion among users.
> > 
> > True.  We need to be careful.
> 
> This could be avoided by using ini-like files (well, easy to read in
> Python) and have [git] (for the common things like author name),
> [cogito], [stgit] etc. sections.

Now if it is going to look like this, I think separate files would be
much more practical, more effective and likely simpler for the user as
well. For Cogito-specific stuff, the user can well dive into
Cogito-specific configuration files, I think. (Well, there's none now;
there is .cgrc but that only contains default options for Cogito
commands and will stay so; I plan ~/.cg/cogito.conf or something.
Actually, perhaps the Git configuration file should be ~/.git/git.conf -
it looks cool, doesn't it?)

> The problem is how much similar we want the Porcelains to be regarding
> the settings and the templates. For StGIT, it is much simpler to have
> something like '%(FILELIST)s' rather than '@FILELIST@' in a template but
> I have not problem with switching to a common syntax. But we should see
> what can easily be changed.

I chose @FILELIST@ only since it is a common convention to have this as
rewrite placeholders, and I think it's more visually clear than
%(FILELIST). Were you insisting on the second syntax, I wouldn't have
%any problem switching, though. Cogito does no @@ rewriting yet.

> I will write a list with what files StGIT uses and where they are placed
> and we can agree on a structure. I think the .git/ directory usage is
> more important to be clarified than having a common {git,cogito,stgit}rc
> file.

Agreed. What Cogito uses:

	.git/author	Default author information in format
				Person Name <email@addy>

	.git/branch-name
			Symbolic name of the branch of this repository.
			This is purely descriptive, does not need to be
			unique and is used only in commit-post. I need
			to distinguish commits done in git-pb and Cogito
			so that's the contents of this file in those two
			repositories. Quite ad-hoc and deserves a better
			solution, but I have none so far; in the future,
			I might just have shared repository for those
			two and use the head name.

	.git/commit-template
			Commit template to use in the commit editor
			instead of some short header (most of it is
			still hardcoded).

	.git/exclude	--exclude-from for git-ls-files
			I want to rename this to .git/ignore

	.git/hooks/commit-post
			COMMIT-ID BRANCHNAME
			(could be <headname> if no branchname defined)

	.git/hooks/merge-pre
			BRANCHNAME BASE CURHEAD MERGEDHEAD MERGETYPE
			MERGETYPE is either "forward" or "tree".
			The merge is cancelled if the script returns
			non-zero exit code.

	.git/hooks/merge-post
			BRANCHNAME BASE CURHEAD MERGEDHEAD MERGETYPE STATUS
			MERGETYPE is either "forward" or "tree".
			For "forward", the STATUS is always "ok",
			while for "tree" the STATUS can be
			"localchanges", "conflicts", "nocommit",
			or "ok".

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
If you want the holes in your knowledge showing up try teaching
someone.  -- Alan Cox

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Catalin Marinas]

On Sat, 2005-07-23 at 11:30 +0200, Petr Baudis wrote:
> Dear diary, on Sat, Jul 23, 2005 at 10:41:38AM CEST, I got a letter
> where Catalin Marinas <catalin.marinas@gmail.com> told me that...
> > The problem appears when one upstream maintainer changes the
> > configuration, should this be merged again? In this case you can get
> > conflicts.
> 
> So you resolve them...? If the upstream keeps doing changes frequent
> enough and large-scale enough to this becoming annoying, something is
> wrong. :-)

OK, that's fine with me (but see below).

> > That's OK with one issue - git should be able to exclude _git when
> > generating a diff between 2 trees, unless one can enforce the _git/*
> > files to be read-only.
> 
> Why? I think those meta-information is important too, and if it differs,
> I want to see it in the diff. Oh, now I see what you mean - to
> optionally exclude it. That would be nice, having --exclude in
> common diff options.

That's useful when generating patches.

The other problem is that the upstream maintainer might not be
interested in my own settings but they would be pulled together with the
normal data.

> > Another option would be to have .git/info/<branch> and, with cogito for
> > example, .git/info/origin should always be pulled, even if the local
> > files were modified. You would override these settings
> > in .git/info/master. The problem is to define the branches order in
> > which the settings are read.
> 
> Yes, and you may be pulling from multiple branches. I would keep
> .git/info simple and single-instanced. If you want your stuff to
> propagate to others, put it to .gitinfo/.

Yes, my idea complicates things quite a lot.

> > This could be avoided by using ini-like files (well, easy to read in
> > Python) and have [git] (for the common things like author name),
> > [cogito], [stgit] etc. sections.
> 
> Now if it is going to look like this, I think separate files would be
> much more practical, more effective and likely simpler for the user as
> well. For Cogito-specific stuff, the user can well dive into
> Cogito-specific configuration files, I think. (Well, there's none now;
> there is .cgrc but that only contains default options for Cogito
> commands and will stay so; I plan ~/.cg/cogito.conf or something.
> Actually, perhaps the Git configuration file should be ~/.git/git.conf -
> it looks cool, doesn't it?)

Or we could have ~/.git/git.conf, ~/.git/cogito.conf and
~/.git/stgit.conf, under the same directory.

> > The problem is how much similar we want the Porcelains to be regarding
> > the settings and the templates. For StGIT, it is much simpler to have
> > something like '%(FILELIST)s' rather than '@FILELIST@' in a template but
> > I have not problem with switching to a common syntax. But we should see
> > what can easily be changed.
> 
> I chose @FILELIST@ only since it is a common convention to have this as
> rewrite placeholders, and I think it's more visually clear than
> %(FILELIST). Were you insisting on the second syntax, I wouldn't have
> %any problem switching, though. Cogito does no @@ rewriting yet.

It's true that @...@ is a common convention and is much clearer. I chose
my syntax since it was easier to format the strings in Python. If we go
for a common template, I would prefer the @...@ one (and do a regexp
replace in Python instead of the string formatting).

> Agreed. What Cogito uses:
> 
> 	.git/author	Default author information in format
> 				Person Name <email@addy>

What about .git/committer? This is useful if I do a commit at work and I
want the repository to have my gmail address.

> 	.git/branch-name
> 			Symbolic name of the branch of this repository.

Isn't this the same as $(readlink .git/HEAD), with some trimming?

> 	.git/commit-template
> 			Commit template to use in the commit editor
> 			instead of some short header (most of it is
> 			still hardcoded).

OK

> 	.git/exclude	--exclude-from for git-ls-files
> 			I want to rename this to .git/ignore

OK, either name is fine with me.

> 	.git/hooks/commit-post
> 	.git/hooks/merge-pre
> 	.git/hooks/merge-post

OK, though StGIT doesn't use any at the moment.

Now, the StGIT files (.git means $GIT_DIR):

      * /etc/stgitrc, ~/.stgitrc, .git/stgitrc - configuration files
        (the latter overrides the former). The syntax is similar to the
        ini files
      * .git/patches/ - directory containing the patch information. I
        won't go into details here since this is only used by StGIT
      * .git/exclude - for the files to be ignored by the 'status'
        command
      * .git/conflicts - includes the list of files conflicting after a
        merge operation. The user should run 'stg resolved --all' to
        mark the conflicts as resolved and remove this file
      * .git/branches/ - the same meaning as in cogito, only that
        'master' is considered a branch and 'stg pull' doesn't use
        'origin'
      * .git/patchdescr.tmpl - the same idea as commit-template, used
        when creating the first description for a patch
      * .git/patchexport.tmpl - template used when exporting the patches
        in a series
      * .git/patchmail.tmpl - template used for sending patches by
        e-mail

-- 
Catalin

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Catalin Marinas]

On Fri, 2005-07-22 at 16:24 -0700, Junio C Hamano wrote:
> Petr Baudis <pasky@suse.cz> writes:
> > This brings me to another subject, M and N are pretty hard to
> > distinguish visually without close inspection of the output. What about
> > switching to use A instead of N everywhere?
> 
> However, I'd like to see what the extent of damage would be even
> if everybody agrees this is a good change.

Using A instead of N is not a problem for StGIT (and I would prefer A
since it's easier to read). I could also only change the StGIT output
even if git uses N.

-- 
Catalin

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Bryan Larsen]

Catalin Marinas wrote:

It seems I inadvertantly kicked off the discussion I wanted to kick off, 
but I didn't excpect this patch to do so!

I prepared a patch adding the following information into 
git/Documentation to kick off discussion.  Obviously Catalin is more 
likely to be accurate.

> 
> OK, though StGIT doesn't use any at the moment.
> 
> Now, the StGIT files (.git means $GIT_DIR):
> 
>       * /etc/stgitrc, ~/.stgitrc, .git/stgitrc - configuration files
>         (the latter overrides the former). The syntax is similar to the
>         ini files
>       * .git/patches/ - directory containing the patch information. I
>         won't go into details here since this is only used by StGIT
>       * .git/exclude - for the files to be ignored by the 'status'
>         command
>       * .git/conflicts - includes the list of files conflicting after a
>         merge operation. The user should run 'stg resolved --all' to
>         mark the conflicts as resolved and remove this file
>       * .git/branches/ - the same meaning as in cogito, only that
>         'master' is considered a branch and 'stg pull' doesn't use
>         'origin'
>       * .git/patchdescr.tmpl - the same idea as commit-template, used
>         when creating the first description for a patch
>       * .git/patchexport.tmpl - template used when exporting the patches
>         in a series
>       * .git/patchmail.tmpl - template used for sending patches by
>         e-mail
> 

how about:
  .git/refs/heads/master - documented in README, doesn't appear to be used.
.git/firstmail.tmpl - template used for sending the preamble email

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Catalin Marinas]

On Sat, 2005-07-23 at 12:33 -0400, Bryan Larsen wrote:
> how about:
>   .git/refs/heads/master - documented in README, doesn't appear to be used.

That's true, README is quite outdated. I created the
http://wiki.procode.org/cgi-bin/wiki.cgi/StGIT page (empty now) where I
will add StGIT information and a tutorial. I will probably keep the
README to a minimum and just point people to the wiki page.

> .git/firstmail.tmpl - template used for sending the preamble email

This file is not used by StGIT. I put it there as an example and you can
use it with the --first option of 'mail'. The reason for this is that
you need to modify this file every time you send a patch series, unlike
the patchmail.tmpl file.

-- 
Catalin

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> I have it the other way around, with the rationale that your default
> settings should be in your ~/.gitrc, not environment, which is always
> the highest priority.

That's true.  I just never hand commit other people's patches (I
use applymbox for that) and never needed to give one-shot set of
environment variables to commit-tree by hand from the command
line.

> (Quite some things came to git from Cogito anyway. ;-) And well, that's
> completely natural.)

I am not the one who did the barebone, so I'd let Linus to tell
"coming from" and "done independently while retaining
compatibility" apart if he wants to ;-).

>> Personally, I think having to have ignore pattern like .cvsignore
>> per-directory is simply _ugly_.
>
> No, I think it's great. That increases the locality of things, which is
> good. Think about it as of variables - it's nicer to have them local.

Seeing Catalin also expressed the intention to add .gitignore in
directory tree everywhere, I would keep my personal opinion to myself.

How about we do something like this:

    git-ls-files --others
        --exclude-from=.git/ignore \
    	--exclude-per-directory=.gitignore

When the new flag --exclude-per-directory is specified,
git-ls-files uses the file with that name in each directory it
looks at to match against the files in that directory (and its
subdirectories, perhaps?)  just like it uses --exclude-from for
the entire tree today.

If I added that, would both of you be able to lose a lot of
lines from cg-status and git.__tree_status()?  If so, then that
is worth the core-side support.

What should the pattern matching rules be?  I think the current
git-ls-files one may be a bit too weak.

[RFC] extending git-ls-files --exclude.

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> Yes. There were several discussions about this in the past, with no
> clear outcome, IIRC. I would prefer:
>
>   ~/.git/ignore per-user
>   /.git/ignore per-repository
>   .gitignore per-directory (cummulative with parent directories)
>
> Note that I also want to make use of some special characters in this
> file ...  to make it at least as powerful as CVS' ignore.

I'd like to extend "--exclude" and friends git-ls-files takes
the following way (strawman).  I'd appreciate your input from
the perspective of Porcelain writers, and somebody who ends up
having to use the bare Plumbing.

I'll be sending patches for actual implementation in separate messages.

------------
'git-ls-files' can use a list of "exclude patterns" when
traversing the directory tree and finding files to show when the
flags --others or --ignored are specified.

These exclude patterns come from these places:

 (1) command line flag --exclude=<pattern> specifies a single
     pattern.

 (2) command line flag --exclude-from=<file> specifies a list of
     patterns stored in a file.

 (3) command line flag --exclude-per-directory=<name> specifies
     a name of the file in each directory 'git-ls-files'
     examines, and if exists, its contents are used as an
     additional list of patterns.

An exclude pattern file used by (2) and (3) contains one pattern
per line.  A line that starts with a '#' can be used as comment
for readability.

The list of patterns that is in effect at a given time is
built and ordered in the following way:

 * --exclude=<pattern> and lines read from --exclude-from=<file>
   come at the beginning of the list of patterns, in the order
   given on the command line.  Patterns that come from the file
   specified with --exclude-from are ordered in the same order
   as they appear in the file.

 * When --exclude-per-directory=<name> is specified, upon
   entering a directory that has such a file, its contents are
   appended at the end of the current "list of patterns".  They
   are popped off when leaving the directory.

Each pattern in the pattern list specifies "a match pattern" and
optionally the fate --- either a file that matches the pattern
is considered excluded or included.  By default, this being
"exclude" mechanism, the fate is "excluded".  A filename is
examined against the patterns in the list, and the first match
determines its fate.

A pattern specified on the command line with --exclude or read
from the file specified with --exclude-from is relative to the
top of the directory tree.  A pattern read from a file specified
by --exclude-per-directory is relative to the directory that the
pattern file appears in.

An exclude pattern is of the following format:

 - an optional prefix '!' which means that the fate this pattern
   specifies is "include", not the usual "exclude"; the
   remainder of the pattern string is interpreted according to
   the following rules.

 - if it does not contain a slash '/', it is a shell glob
   pattern and used to match against the filename without
   leading directories (i.e. the same way as the current
   implementation).

 - otherwise, it is a shell glob pattern, suitable for
   consumption by fnmatch(3) with FNM_PATHNAME flag.  I.e. a
   slash in the pattern must match a slash in the pathname.
   "Documentation/*.html" matches "Documentation/git.html" but
   not "ppc/ppc.html".  As a natural exception, "/*.c" matches
   "cat-file.c" but not "mozilla-sha1/sha1.c".

An example: 

    $ cat .git/ignore
    # ignore objects and archives, anywhere in the tree.
    *.[oa]
    $ cat Documentation/.gitignore
    # ignore generated html files,
    # except foo.html which is maintained by hand
    !foo.html
    *.html
    $ git-ls-files --ignored \
        --exclude='Documentation/*.[0-9]' \
        --exclude-from=.git/ignore \
        --exclude-per-directory=.gitignore

[PATCH] git-ls-files: --exclude mechanism updates.

[Junio C Hamano]

Add --exclude-per-directory=<name> option that specifies a file
to contain exclude patterns local to that directory and its
subdirectories.  Update the exclusion logic to be able to say
"include files that match this more specific pattern, even
though later exclude patterns may match them".  Also enhances
that a pattern can contain '/' in which case fnmatch is called
with FNM_PATHNAME flag to match the entire path.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

 ls-files.c                         |  123 ++++++++++++++++++++++++++++++------
 t/t3001-ls-files-others-exclude.sh |   55 ++++++++++++++++
 2 files changed, 157 insertions(+), 21 deletions(-)
 create mode 100755 t/t3001-ls-files-others-exclude.sh

d1466fd8701ca79a91b41c6225c115a0a9866d6e
diff --git a/ls-files.c b/ls-files.c
--- a/ls-files.c
+++ b/ls-files.c
@@ -25,20 +25,31 @@ static const char *tag_removed = "";
 static const char *tag_other = "";
 static const char *tag_killed = "";
 
+static char *exclude_per_dir = NULL;
 static int nr_excludes;
-static const char **excludes;
 static int excludes_alloc;
+static struct exclude {
+	const char *pattern;
+	const char *base;
+	int baselen;
+} **excludes;
 
-static void add_exclude(const char *string)
+static void add_exclude(const char *string, const char *base, int baselen)
 {
+	struct exclude *x = xmalloc(sizeof (*x));
+
+	x->pattern = string;
+	x->base = base;
+	x->baselen = baselen;
 	if (nr_excludes == excludes_alloc) {
 		excludes_alloc = alloc_nr(excludes_alloc);
 		excludes = realloc(excludes, excludes_alloc*sizeof(char *));
 	}
-	excludes[nr_excludes++] = string;
+	excludes[nr_excludes++] = x;
 }
 
-static void add_excludes_from_file(const char *fname)
+static int add_excludes_from_file_1(const char *fname,
+				    const char *base, int baselen)
 {
 	int fd, i;
 	long size;
@@ -53,7 +64,7 @@ static void add_excludes_from_file(const
 	lseek(fd, 0, SEEK_SET);
 	if (size == 0) {
 		close(fd);
-		return;
+		return 0;
 	}
 	buf = xmalloc(size);
 	if (read(fd, buf, size) != size)
@@ -63,28 +74,89 @@ static void add_excludes_from_file(const
 	entry = buf;
 	for (i = 0; i < size; i++) {
 		if (buf[i] == '\n') {
-			if (entry != buf + i) {
+			if (entry != buf + i && entry[0] != '#') {
 				buf[i] = 0;
-				add_exclude(entry);
+				add_exclude(entry, base, baselen);
 			}
 			entry = buf + i + 1;
 		}
 	}
-	return;
+	return 0;
 
-err:	perror(fname);
-	exit(1);
+ err:
+	if (0 <= fd)
+		close(fd);
+	return -1;
+}
+
+static void add_excludes_from_file(const char *fname)
+{
+	if (add_excludes_from_file_1(fname, "", 0) < 0)
+		die("cannot use %s as an exclude file", fname);
+}
+
+static int push_exclude_per_directory(const char *base, int baselen)
+{
+	char exclude_file[PATH_MAX];
+	int current_nr = nr_excludes;
+
+	if (exclude_per_dir) {
+		memcpy(exclude_file, base, baselen);
+		strcpy(exclude_file + baselen, exclude_per_dir);
+		add_excludes_from_file_1(exclude_file, base, baselen);
+	}
+	return current_nr;
+}
+
+static void pop_exclude_per_directory(int stk)
+{
+	while (stk < nr_excludes)
+		free(excludes[--nr_excludes]);
 }
 
 static int excluded(const char *pathname)
 {
 	int i;
+
 	if (nr_excludes) {
-		const char *basename = strrchr(pathname, '/');
-		basename = (basename) ? basename+1 : pathname;
-		for (i = 0; i < nr_excludes; i++)
-			if (fnmatch(excludes[i], basename, 0) == 0)
-				return 1;
+		int pathlen = strlen(pathname);
+
+		for (i = 0; i < nr_excludes; i++) {
+			struct exclude *x = excludes[i];
+			const char *exclude = x->pattern;
+			int to_exclude = 1;
+
+			if (*exclude == '!') {
+				to_exclude = 0;
+				exclude++;
+			}
+
+			if (!strchr(exclude, '/')) {
+				/* match basename */
+				const char *basename = strrchr(pathname, '/');
+				basename = (basename) ? basename+1 : pathname;
+				if (fnmatch(exclude, basename, 0) == 0)
+					return to_exclude;
+			}
+			else {
+				/* match with FNM_PATHNAME:
+				 * exclude has base (baselen long) inplicitly
+				 * in front of it.
+				 */
+				int baselen = x->baselen;
+				if (*exclude == '/')
+					exclude++;
+
+				if (pathlen < baselen ||
+				    (baselen && pathname[baselen-1] != '/') ||
+				    strncmp(pathname, x->base, baselen))
+				    continue;
+
+				if (fnmatch(exclude, pathname+baselen,
+					    FNM_PATHNAME) == 0)
+					return to_exclude;
+			}
+		}
 	}
 	return 0;
 }
@@ -121,7 +193,7 @@ static void add_name(const char *pathnam
  * doesn't handle them at all yet. Maybe that will change some
  * day.
  *
- * Also, we currently ignore all names starting with a dot.
+ * Also, we ignore the name ".git" (even if it is not a directory).
  * That likely will not change.
  */
 static void read_directory(const char *path, const char *base, int baselen)
@@ -129,10 +201,13 @@ static void read_directory(const char *p
 	DIR *dir = opendir(path);
 
 	if (dir) {
+		int exclude_stk;
 		struct dirent *de;
 		char fullname[MAXPATHLEN + 1];
 		memcpy(fullname, base, baselen);
 
+		exclude_stk = push_exclude_per_directory(base, baselen);
+
 		while ((de = readdir(dir)) != NULL) {
 			int len;
 
@@ -141,10 +216,10 @@ static void read_directory(const char *p
 			     !strcmp(de->d_name + 1, ".") ||
 			     !strcmp(de->d_name + 1, "git")))
 				continue;
-			if (excluded(de->d_name) != show_ignored)
-				continue;
 			len = strlen(de->d_name);
 			memcpy(fullname + baselen, de->d_name, len+1);
+			if (excluded(fullname) != show_ignored)
+				continue;
 
 			switch (DTYPE(de)) {
 			struct stat st;
@@ -170,6 +245,8 @@ static void read_directory(const char *p
 			add_name(fullname, baselen + len);
 		}
 		closedir(dir);
+
+		pop_exclude_per_directory(exclude_stk);
 	}
 }
 
@@ -287,7 +364,9 @@ static void show_files(void)
 
 static const char *ls_files_usage =
 	"git-ls-files [-z] [-t] (--[cached|deleted|others|stage|unmerged|killed])* "
-	"[ --ignored [--exclude=<pattern>] [--exclude-from=<file>) ]";
+	"[ --ignored ] [--exclude=<pattern>] [--exclude-from=<file>] "
+	"[ --exclude-per-directory=<filename> ]";
+;
 
 int main(int argc, char **argv)
 {
@@ -323,13 +402,15 @@ int main(int argc, char **argv)
 			show_stage = 1;
 			show_unmerged = 1;
 		} else if (!strcmp(arg, "-x") && i+1 < argc) {
-			add_exclude(argv[++i]);
+			add_exclude(argv[++i], "", 0);
 		} else if (!strncmp(arg, "--exclude=", 10)) {
-			add_exclude(arg+10);
+			add_exclude(arg+10, "", 0);
 		} else if (!strcmp(arg, "-X") && i+1 < argc) {
 			add_excludes_from_file(argv[++i]);
 		} else if (!strncmp(arg, "--exclude-from=", 15)) {
 			add_excludes_from_file(arg+15);
+		} else if (!strncmp(arg, "--exclude-per-directory=", 24)) {
+			exclude_per_dir = arg + 24;
 		} else
 			usage(ls_files_usage);
 	}
diff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh
new file mode 100755
--- /dev/null
+++ b/t/t3001-ls-files-others-exclude.sh
@@ -0,0 +1,55 @@
+#!/bin/sh
+#
+# Copyright (c) 2005 Junio C Hamano
+#
+
+test_description='git-ls-files --others --exclude
+
+This test runs git-ls-files --others and tests --exclude patterns.
+'
+
+. ./test-lib.sh
+
+rm -fr one three
+for dir in . one one/two three
+do
+  mkdir -p $dir &&
+  for i in 1 2 3 4 5
+  do
+    >$dir/a.$i
+  done
+done
+
+cat >expect <<EOF
+a.2
+a.4
+a.5
+one/a.3
+one/a.4
+one/a.5
+one/two/a.3
+one/two/a.5
+three/a.2
+three/a.3
+three/a.4
+three/a.5
+EOF
+
+echo '.gitignore
+output
+expect
+.gitignore
+' >.git/ignore
+
+echo '*.1
+/*.3' >.gitignore
+echo '*.2
+two/*.4' >one/.gitignore
+
+test_expect_success \
+    'git-ls-files --others --exclude.' \
+    'git-ls-files --others \
+       --exclude-per-directory=.gitignore \
+       --exclude-from=.git/ignore \
+       >output &&
+     diff -u expect output'

[PATCH] Documentation: describe git-ls-files --exclude patterns.

[Junio C Hamano]

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

 Documentation/git-ls-files.txt |   96 ++++++++++++++++++++++++++++++++++++++--
 1 files changed, 92 insertions(+), 4 deletions(-)

d9296497b70d9007da94cec453ecb5c6c7173140
diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
--- a/Documentation/git-ls-files.txt
+++ b/Documentation/git-ls-files.txt
@@ -14,6 +14,7 @@ SYNOPSIS
 		(-[c|d|o|i|s|u|k])\*
 		[-x <pattern>|--exclude=<pattern>]
 		[-X <file>|--exclude-from=<file>]
+		[--exclude-per-directory=<file>]
 
 DESCRIPTION
 -----------
@@ -59,10 +60,10 @@ OPTIONS
 
 -X|--exclude-from=<file>::
 	exclude patterns are read from <file>; 1 per line.
-	Allows the use of the famous dontdiff file as follows to find
-	out about uncommitted files just as dontdiff is used with
-	the diff command:
-	     git-ls-files --others --exclude-from=dontdiff
+
+--exclude-per-directory=<file>::
+	read additional exclude patterns that apply only to the
+	directory and its subdirectories in <file>.
 
 -t::
 	Identify the file status with the following tags (followed by
@@ -89,6 +90,93 @@ the dircache records up to three such pa
 the user (or Cogito) to see what should eventually be recorded at the
 path. (see read-cache for more information on state)
 
+
+Exclude Patterns
+----------------
+
+'git-ls-files' can use a list of "exclude patterns" when
+traversing the directory tree and finding files to show when the
+flags --others or --ignored are specified.
+
+These exclude patterns come from these places:
+
+ (1) command line flag --exclude=<pattern> specifies a single
+     pattern.
+
+ (2) command line flag --exclude-from=<file> specifies a list of
+     patterns stored in a file.
+
+ (3) command line flag --exclude-per-directory=<name> specifies
+     a name of the file in each directory 'git-ls-files'
+     examines, and if exists, its contents are used as an
+     additional list of patterns.
+
+An exclude pattern file used by (2) and (3) contains one pattern
+per line.  A line that starts with a '#' can be used as comment
+for readability.
+
+The list of patterns that is in effect at a given time is
+built and ordered in the following way:
+
+ * --exclude=<pattern> and lines read from --exclude-from=<file>
+   come at the beginning of the list of patterns, in the order
+   given on the command line.  Patterns that come from the file
+   specified with --exclude-from are ordered in the same order
+   as they appear in the file.
+
+ * When --exclude-per-directory=<name> is specified, upon
+   entering a directory that has such a file, its contents are
+   appended at the end of the current "list of patterns".  They
+   are popped off when leaving the directory.
+
+Each pattern in the pattern list specifies "a match pattern" and
+optionally the fate --- either a file that matches the pattern
+is considered excluded or included.  By default, this being
+"exclude" mechanism, the fate is "excluded".  A filename is
+examined against the patterns in the list, and the first match
+determines its fate.
+
+A pattern specified on the command line with --exclude or read
+from the file specified with --exclude-from is relative to the
+top of the directory tree.  A pattern read from a file specified
+by --exclude-per-directory is relative to the directory that the
+pattern file appears in.
+
+An exclude pattern is of the following format:
+
+ - an optional prefix '!' which means that the fate this pattern
+   specifies is "include", not the usual "exclude"; the
+   remainder of the pattern string is interpreted according to
+   the following rules.
+
+ - if it does not contain a slash '/', it is a shell glob
+   pattern and used to match against the filename without
+   leading directories (i.e. the same way as the current
+   implementation).
+
+ - otherwise, it is a shell glob pattern, suitable for
+   consumption by fnmatch(3) with FNM_PATHNAME flag.  I.e. a
+   slash in the pattern must match a slash in the pathname.
+   "Documentation/*.html" matches "Documentation/git.html" but
+   not "ppc/ppc.html".  As a natural exception, "/*.c" matches
+   "cat-file.c" but not "mozilla-sha1/sha1.c".
+
+An example:
+
+    $ cat .git/ignore
+    # ignore objects and archives, anywhere in the tree.
+    *.[oa]
+    $ cat Documentation/.gitignore
+    # ignore generated html files,
+    # except foo.html which is maintained by hand
+    !foo.html
+    *.html
+    $ git-ls-files --ignored \
+        --exclude='Documentation/*.[0-9]' \
+        --exclude-from=.git/ignore \
+        --exclude-per-directory=.gitignore
+
+
 See Also
 --------
 link:read-cache.html[read-cache]

Re: [RFC] extending git-ls-files --exclude.

[Marco Costalba]

Junio C Hamano wrote:


>The list of patterns that is in effect at a given time is
>built and ordered in the following way:
>
> * --exclude=<pattern> and lines read from --exclude-from=<file>
>   come at the beginning of the list of patterns, in the order
>   given on the command line.  Patterns that come from the file
>   specified with --exclude-from are ordered in the same order
>   as they appear in the file.
>
> * When --exclude-per-directory=<name> is specified, upon
>   entering a directory that has such a file, its contents are
>   appended at the end of the current "list of patterns".  They
>   are popped off when leaving the directory.
>

Are really necessary to have both --exclude-from=<file> and 
--exclude-per-directory=<name> ?

Peraphs, if the file name of excluded list is the same for each directory, 
e.g. .gitignore or something similar, instead of --exclude-per-directory
we can use a concept of file validity 'scope' and just use --exclude-from=<file>.
If entering in a directory <file> is found its contents are appended and 
removed when leaving directory. A bad analogy can be with the use of 
recursive Makefile.

>
>A pattern specified on the command line with --exclude or read
>from the file specified with --exclude-from is relative to the
>top of the directory tree.  A pattern read from a file specified
>by --exclude-per-directory is relative to the directory that the
>pattern file appears in.
>

If we use the 'scope' logic we can just prepend path when adding entries
and serach with with FNM_PATHNAME flag.

> - if it does not contain a slash '/', it is a shell glob
>   pattern and used to match against the filename without
>   leading directories (i.e. the same way as the current
>   implementation).
>
> - otherwise, it is a shell glob pattern, suitable for
>   consumption by fnmatch(3) with FNM_PATHNAME flag.  I.e. a
>   slash in the pattern must match a slash in the pathname.
>   "Documentation/*.html" matches "Documentation/git.html" but
>   not "ppc/ppc.html".  As a natural exception, "/*.c" matches
>   "cat-file.c" but not "mozilla-sha1/sha1.c".
>

Same comment as above, if prepending path when adding per 
directory contents we can simplify to always use FNM_PATHNAME flag.

We don't have even to special case base directory global
scope <file> if we use realtive paths so that we prepend its contents 
with ./ and we have what we expect.

I am sorry to not be able to send a patch, better explaining what 
proposed but I am just leaving today and I will be off line
for a couple of weeks.


Marco


__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 

Re: [RFC] extending git-ls-files --exclude.

[Junio C Hamano]

Marco Costalba <mcostalba@yahoo.it> writes:

> Are really necessary to have both --exclude-from=<file> and 
> --exclude-per-directory=<name> ?
>
> Peraphs, if the file name of excluded list is the same for each directory, 
> e.g. .gitignore or something similar, instead of --exclude-per-directory
> we can use a concept of file validity 'scope' and just use --exclude-from=<file>.
> If entering in a directory <file> is found its contents are appended and 
> removed when leaving directory. A bad analogy can be with the use of 
> recursive Makefile.

Pasky wants to have one of the files in ~/.git-something if I
understand correctly.  I did not want to remove --exclude-from
for that reason.

> If we use the 'scope' logic we can just prepend path when
> adding entries and serach with with FNM_PATHNAME flag.

And I wanted for people's script and existing ignore files to
continue to work.

> Same comment as above, if prepending path when adding per 
> directory contents we can simplify to always use FNM_PATHNAME flag.

Yes, but that means you need to always prepend the current
directory being looked at to get the original behaviour.  The
way it was originally done without FNM_PATHNAME was far easier
to read, at least for me.

Re: [RFC] extending git-ls-files --exclude.

[Catalin Marinas]

Junio C Hamano <junkio@cox.net> wrote:
>  * When --exclude-per-directory=<name> is specified, upon
>    entering a directory that has such a file, its contents are
>    appended at the end of the current "list of patterns".  They
>    are popped off when leaving the directory.
[...]
> A pattern specified on the command line with --exclude or read
> from the file specified with --exclude-from is relative to the
> top of the directory tree.  A pattern read from a file specified
> by --exclude-per-directory is relative to the directory that the
> pattern file appears in.

I think it would make more sense for the exclude-per-directory
patterns to be local to that directory only, without recursively
preserving them for subdirectories. One would, in general, put the
common exclude patterns like *.o *~ etc. in the global file
(.git/exclude). The patterns local to a directory only (take the
vmlinux file for example), one would write it in the .gitignore file
but this should be used for subdirectories.

> An exclude pattern is of the following format:
[...]

That's fine. Actually, the Porcelain would care much about it since it
gets the information already filtered by git.

>     $ cat Documentation/.gitignore
>     # ignore generated html files,
>     # except foo.html which is maintained by hand
>     !foo.html
>     *.html

Wouldn't it be clearer to have the general rules first (*.html),
overridden by the more specific ones (!foo.html)? Just my opinion, I
don't know what others think.

-- 
Catalin

Re: [RFC] extending git-ls-files --exclude.

[Junio C Hamano]

Marco Costalba <mcostalba@yahoo.it> writes:

> Peraphs, if the file name of excluded list is the same for each directory, 
> e.g. .gitignore or something similar, instead of --exclude-per-directory
> we can use a concept of file validity 'scope' and just use --exclude-from=<file>.
> If entering in a directory <file> is found its contents are appended and 
> removed when leaving directory. A bad analogy can be with the use of 
> recursive Makefile.

I do not know if you noticed it, but the patch already have the
concept of "scope".  If a pattern has a slash '/' it is scoped
to the named directory by using FNM_PATHNAME.  "The named
directory" for command line --exclude and --exclude-from are
relative to the project top while it is relative to the
directory the file being used is in for --exclude-per-directory
case.

> If we use the 'scope' logic we can just prepend path when adding entries
> and serach with with FNM_PATHNAME flag.

You suggest to always use FNM_PATHNAME by adjusting the pattern
by prefixing the scope.  While that can theoretically be made to
work just as the posted patch does, you need to realize it is a
lot more cumbersome to deal with metacharacters in pathnames
your way.  If the prefix you need to add (because you are now
looking at a path for that directory which has a funny name) is
"foo*bar", you need to add "foo\*bar" to the pattern in order to
make sure that the asterisk is not used to match anything other
than a literal asterisk, for example.

You could argue that nobody should use funny characters in
pathnames, and you could further argue that current Porcelains
do not handle certain characters in pathnames anyway so why
bother.  But ls-files being core, which is "the funny filesystem
layer to build SCM on", I would like to be careful not to be
unnecessarily restrictive.

Re: [RFC] extending git-ls-files --exclude.

[Junio C Hamano]

Catalin Marinas <catalin.marinas@gmail.com> writes:

> I think it would make more sense for the exclude-per-directory
> patterns to be local to that directory only, without recursively
> preserving them for subdirectories.

I personally do not have preference either way, but am slightly
biased towards the "cumulative" behaviour the patch attempts to
implement, which was what Pasky said he wanted to have.

    Date: Fri, 22 Jul 2005 22:59:48 +0200
    From: Petr Baudis <pasky@suse.cz>
    Subject: Re: [PATCH 1/1] Tell vim the textwidth is 75.
    Message-ID: <20050722205948.GE11916@pasky.ji.cz>

    > *3* .gitignore in the cwd is used in Cogito, if I am not
    > mistaken.

    Yes. There were several discussions about this in the past, with no
    clear outcome, IIRC. I would prefer:

      ~/.git/ignore per-user
      /.git/ignore per-repository
      .gitignore per-directory (cummulative with parent directories)

>> An exclude pattern is of the following format:
> [...]
>
> That's fine. Actually, the Porcelain would care much about it since it
> gets the information already filtered by git.

Your saying "fine" is a relief.  This change aims at helping
Porcelain people by making it less likely for Porcelain to need
its own filtering.  As you say, if ls-files filters more than
the Porcelain wants, that's a bigger problem.

>>     $ cat Documentation/.gitignore
>>     # ignore generated html files,
>>     # except foo.html which is maintained by hand
>>     !foo.html
>>     *.html
>
> Wouldn't it be clearer to have the general rules first (*.html),
> overridden by the more specific ones (!foo.html)? Just my opinion, I
> don't know what others think.

I do not know, either, but I do know it is consistent with the
"first match determines fate" rule and cleaner to implement.

Re: [RFC] extending git-ls-files --exclude.

[Linus Torvalds]

On Mon, 25 Jul 2005, Junio C Hamano wrote:
> 
> I personally do not have preference either way, but am slightly
> biased towards the "cumulative" behaviour the patch attempts to
> implement, which was what Pasky said he wanted to have.

I think that makes sense.

Imagine, for example, that you have separate subdirectory structures for 
Documentation and for source - maybe you'd put the "*.o" rule in the 
source directory, and a "*.1" rule in the Docs subdirectory.

			Linus

Re: [RFC] extending git-ls-files --exclude.

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> On Mon, 25 Jul 2005, Junio C Hamano wrote:
>> 
>> I personally do not have preference either way, but am slightly
>> biased towards the "cumulative" behaviour the patch attempts to
>> implement, which was what Pasky said he wanted to have.
>
> I think that makes sense.
>
> Imagine, for example, that you have separate subdirectory structures for 
> Documentation and for source - maybe you'd put the "*.o" rule in the 
> source directory, and a "*.1" rule in the Docs subdirectory.

I imagined it, but it appears to me that this is a bad example.
My understanding of what Catalin and the proposed patch
disagrees is whether the patterns in .gitignore at the top level
should govern files under ppc/ and mozilla-sha1/ subdirectories;
Catalin thinks they should not.

What I meant by "cumulative" (now I realize I might have
misunderstood what Pasky wanted to mean by that word, though)
was not just .gitignore in subdirectory being added, but the
effect of patterns being added so far, either from the command
line or by parent directories, last while in the deeper
directories.

Re: [RFC] extending git-ls-files --exclude.

[Catalin Marinas]

On Mon, 2005-07-25 at 13:27 -0700, Junio C Hamano wrote:
> Linus Torvalds <torvalds@osdl.org> writes:
> > Imagine, for example, that you have separate subdirectory structures for 
> > Documentation and for source - maybe you'd put the "*.o" rule in the 
> > source directory, and a "*.1" rule in the Docs subdirectory.
> 
> I imagined it, but it appears to me that this is a bad example.
> My understanding of what Catalin and the proposed patch
> disagrees is whether the patterns in .gitignore at the top level
> should govern files under ppc/ and mozilla-sha1/ subdirectories;
> Catalin thinks they should not.

I really don't have a strong preference for this. Linus' example makes
sense as well. It's up to you how you implement its behaviour,
Porcelains shouldn't be affected by this.

Since I'm the only one with this idea, you can forget about it (I don't
mind :-) ). If others express a preference for this, you could implement
a 'cut' label (similar to Prolog's cut operator) which clears the
pattern stack before diving into subdirectories. Anyway, I don't think
it's worse the hassle since it might never be used.

-- 
Catalin

Re: [RFC] extending git-ls-files --exclude.

[Catalin Marinas]

On Mon, 2005-07-25 at 12:58 -0700, Junio C Hamano wrote:
> Catalin Marinas <catalin.marinas@gmail.com> writes:
> >> An exclude pattern is of the following format:
> > [...]
> >
> > That's fine. Actually, the Porcelain would care much about it since it
> > gets the information already filtered by git.
> 
> Your saying "fine" is a relief.  This change aims at helping
> Porcelain people by making it less likely for Porcelain to need
> its own filtering. As you say, if ls-files filters more than
> the Porcelain wants, that's a bigger problem.

I don't plan to add any additional filtering in StGIT. What I meant
above was that Porcelain would not care much about the patterns. The
user should cope with what git provides, nothing more. With these git
patches, I think there are enough features for filtering.

> > Wouldn't it be clearer to have the general rules first (*.html),
> > overridden by the more specific ones (!foo.html)? Just my opinion, I
> > don't know what others think.
> 
> I do not know, either, but I do know it is consistent with the
> "first match determines fate" rule and cleaner to implement.

I also don't have a strong preference for this and the "first match"
rule clarifies it (otherwise, you could have pushed them on a list in
reverse order).

-- 
Catalin

Re: [RFC] extending git-ls-files --exclude.

[Junio C Hamano]

Marco Costalba <mcostalba@yahoo.it> writes:

> This cumulative effect brakes local scoping of .gitignore files in 
> corresponding subdirectory.
>
> As example in a directory A we can have a .gitignore file with
>
> !foo.html
> *.html
>
> because we want to special case 'that' foo.html in 'that'
> directory.

I guess that !foo.html I wrote was a bad example of special case
not being specific enough.  Saying "!/foo.html" should work
[*1*], so I think "cumulative effect _breaks_" is a bit too strong
a word.

Having said that,...

> This can be powerful and flexible but also prone to errors. 

While I also suspect it _might_ be prone to user errors, at
least to some classes of users, Catalin (and Peter as well I
suspect, although I may be mistaken by what he originally meant
by "cumulative") seems to think it is OK.  As a developer of
another Porcelain (qgit), your input is as valuable as others,
so...

BTW, I am CC'ing git list because I saw you forwarded your
entire message to the list as a quoted message form.  Please do
not do that.  I cannot distinguish a message like that with a
message with nothing but quote and no original contents, which I
normally discard without even reading.

[Footnote]

*1* I just tried, and it seems to work.

  $ rm -f .gitignore ppc/.gitignore
  $ (echo '!/foo.bar'; echo '*.bar') >.gitignore
  $ for i in foo.bar baz.bar; do date >$i; date >ppc/$i; done
  $ git-ls-files --others --exclude-per-directory=.gitignore | grep '\.bar'
  foo.bar
  $ rm -f .gitignore ppc/.gitignore

Updating diff-raw status letter to 'A' for added files.

[Junio C Hamano]

Since both of you seem to be in favor of using 'A' instead of
'N' for added files in the diff-raw output, here are two patches
requesting for test.

  [PATCH] Use symbolic constants for diff-raw status indicators.
  [PATCH] diff-raw: Use 'A' instead of 'N' for added files.

[PATCH 1/2] Use symbolic constants for diff-raw status indicators.

[Junio C Hamano]

Both Cogito and StGIT prefer to see 'A' for new files.  The
current 'N' is visually harder to distinguish from 'M', which is
used for modified files.  Prepare the internals to use symbolic
constants to make the change easier.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

 diff-helper.c |    3 ++-
 diff.c        |   58 ++++++++++++++++++++++++++++++++-------------------------
 diff.h        |   16 ++++++++++++++++
 3 files changed, 51 insertions(+), 26 deletions(-)

e7baa4f45f4420a6d2da6a13e8959f8405c3ea19
diff --git a/diff-helper.c b/diff-helper.c
--- a/diff-helper.c
+++ b/diff-helper.c
@@ -94,7 +94,8 @@ int main(int ac, const char **av) {
 			if (!strchr("MCRNDU", status))
 				break;
 			two_paths = score = 0;
-			if (status == 'R' || status == 'C')
+			if (status == DIFF_STATUS_RENAMED ||
+			    status == DIFF_STATUS_COPIED)
 				two_paths = 1;
 
 			/* pick up score if exists */
diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -617,7 +617,7 @@ static void run_diff(struct diff_filepai
 	other = (strcmp(name, p->two->path) ? p->two->path : NULL);
 	one = p->one; two = p->two;
 	switch (p->status) {
-	case 'C':
+	case DIFF_STATUS_COPIED:
 		sprintf(msg_,
 			"similarity index %d%%\n"
 			"copy from %s\n"
@@ -626,7 +626,7 @@ static void run_diff(struct diff_filepai
 			name, other);
 		xfrm_msg = msg_;
 		break;
-	case 'R':
+	case DIFF_STATUS_RENAMED:
 		sprintf(msg_,
 			"similarity index %d%%\n"
 			"rename from %s\n"
@@ -635,7 +635,7 @@ static void run_diff(struct diff_filepai
 			name, other);
 		xfrm_msg = msg_;
 		break;
-	case 'M':
+	case DIFF_STATUS_MODIFIED:
 		if (p->score) {
 			sprintf(msg_,
 				"dissimilarity index %d%%",
@@ -796,10 +796,12 @@ static void diff_flush_raw(struct diff_f
 		status[1] = 0;
 	}
 	switch (p->status) {
-	case 'C': case 'R':
+	case DIFF_STATUS_COPIED:
+	case DIFF_STATUS_RENAMED:
 		two_paths = 1;
 		break;
-	case 'N': case 'D':
+	case DIFF_STATUS_ADDED:
+	case DIFF_STATUS_DELETED:
 		two_paths = 0;
 		break;
 	default:
@@ -928,13 +930,13 @@ static void diff_resolve_rename_copy(voi
 		p = q->queue[i];
 		p->status = 0; /* undecided */
 		if (DIFF_PAIR_UNMERGED(p))
-			p->status = 'U';
+			p->status = DIFF_STATUS_UNMERGED;
 		else if (!DIFF_FILE_VALID(p->one))
-			p->status = 'N';
+			p->status = DIFF_STATUS_ADDED;
 		else if (!DIFF_FILE_VALID(p->two))
-			p->status = 'D';
+			p->status = DIFF_STATUS_DELETED;
 		else if (DIFF_PAIR_TYPE_CHANGED(p))
-			p->status = 'T';
+			p->status = DIFF_STATUS_TYPE_CHANGED;
 
 		/* from this point on, we are dealing with a pair
 		 * whose both sides are valid and of the same type, i.e.
@@ -942,7 +944,7 @@ static void diff_resolve_rename_copy(voi
 		 */
 		else if (DIFF_PAIR_RENAME(p)) {
 			if (p->source_stays) {
-				p->status = 'C';
+				p->status = DIFF_STATUS_COPIED;
 				continue;
 			}
 			/* See if there is some other filepair that
@@ -956,22 +958,22 @@ static void diff_resolve_rename_copy(voi
 				if (!DIFF_PAIR_RENAME(pp))
 					continue; /* not a rename/copy */
 				/* pp is a rename/copy from the same source */
-				p->status = 'C';
+				p->status = DIFF_STATUS_COPIED;
 				break;
 			}
 			if (!p->status)
-				p->status = 'R';
+				p->status = DIFF_STATUS_RENAMED;
 		}
 		else if (memcmp(p->one->sha1, p->two->sha1, 20) ||
 			 p->one->mode != p->two->mode)
-			p->status = 'M';
+			p->status = DIFF_STATUS_MODIFIED;
 		else {
 			/* This is a "no-change" entry and should not
 			 * happen anymore, but prepare for broken callers.
 			 */
 			error("feeding unmodified %s to diffcore",
 			      p->one->path);
-			p->status = 'X';
+			p->status = DIFF_STATUS_UNKNOWN;
 		}
 	}
 	diff_debug_queue("resolve-rename-copy done", q);
@@ -989,7 +991,7 @@ void diff_flush(int diff_output_style, i
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
 		if ((diff_output_style == DIFF_FORMAT_NO_OUTPUT) ||
-		    (p->status == 'X'))
+		    (p->status == DIFF_STATUS_UNKNOWN))
 			continue;
 		if (p->status == 0)
 			die("internal error in diff-resolve-rename-copy");
@@ -1024,15 +1026,17 @@ static void diffcore_apply_filter(const 
 	if (!filter)
 		return;
 
-	if (strchr(filter, 'A')) {
-		/* All-or-none */
+	if (strchr(filter, DIFF_STATUS_FILTER_AON)) {
 		int found;
 		for (i = found = 0; !found && i < q->nr; i++) {
 			struct diff_filepair *p = q->queue[i];
-			if (((p->status == 'M') &&
-			     ((p->score && strchr(filter, 'B')) ||
-			      (!p->score && strchr(filter, 'M')))) ||
-			    ((p->status != 'M') && strchr(filter, p->status)))
+			if (((p->status == DIFF_STATUS_MODIFIED) &&
+			     ((p->score &&
+			       strchr(filter, DIFF_STATUS_FILTER_BROKEN)) ||
+			      (!p->score &&
+			       strchr(filter, DIFF_STATUS_MODIFIED)))) ||
+			    ((p->status != DIFF_STATUS_MODIFIED) &&
+			     strchr(filter, p->status)))
 				found++;
 		}
 		if (found)
@@ -1050,10 +1054,14 @@ static void diffcore_apply_filter(const 
 		/* Only the matching ones */
 		for (i = 0; i < q->nr; i++) {
 			struct diff_filepair *p = q->queue[i];
-			if (((p->status == 'M') &&
-			     ((p->score && strchr(filter, 'B')) ||
-			      (!p->score && strchr(filter, 'M')))) ||
-			    ((p->status != 'M') && strchr(filter, p->status)))
+
+			if (((p->status == DIFF_STATUS_MODIFIED) &&
+			     ((p->score &&
+			       strchr(filter, DIFF_STATUS_FILTER_BROKEN)) ||
+			      (!p->score &&
+			       strchr(filter, DIFF_STATUS_MODIFIED)))) ||
+			    ((p->status != DIFF_STATUS_MODIFIED) &&
+			     strchr(filter, p->status)))
 				diff_q(&outq, p);
 			else
 				diff_free_filepair(p);
diff --git a/diff.h b/diff.h
--- a/diff.h
+++ b/diff.h
@@ -82,4 +82,20 @@ extern int diff_queue_is_empty(void);
 
 extern void diff_flush(int output_style, int line_terminator);
 
+/* diff-raw status letters */
+#define DIFF_STATUS_ADDED		'N'
+#define DIFF_STATUS_COPIED		'C'
+#define DIFF_STATUS_DELETED		'D'
+#define DIFF_STATUS_MODIFIED		'M'
+#define DIFF_STATUS_RENAMED		'R'
+#define DIFF_STATUS_TYPE_CHANGED	'T'
+#define DIFF_STATUS_UNKNOWN		'X'
+#define DIFF_STATUS_UNMERGED		'U'
+
+/* these are not diff-raw status letters proper, but used by
+ * diffcore-filter insn to specify additional restrictions.
+ */
+#define DIFF_STATUS_FILTER_AON		'A'
+#define DIFF_STATUS_FILTER_BROKEN	'B'
+
 #endif /* DIFF_H */

[PATCH 2/2] diff-raw: Use 'A' instead of 'N' for added files.

[Junio C Hamano]

This actually changes the diff-raw status letter from N to A
for added files.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

 diff.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

ca8c9156f8f980455f64e2cedcf0347328e46029
diff --git a/diff.h b/diff.h
--- a/diff.h
+++ b/diff.h
@@ -83,7 +83,7 @@ extern int diff_queue_is_empty(void);
 extern void diff_flush(int output_style, int line_terminator);
 
 /* diff-raw status letters */
-#define DIFF_STATUS_ADDED		'N'
+#define DIFF_STATUS_ADDED		'A'
 #define DIFF_STATUS_COPIED		'C'
 #define DIFF_STATUS_DELETED		'D'
 #define DIFF_STATUS_MODIFIED		'M'

Re: [RFC] extending git-ls-files --exclude.

[Petr Baudis]

  Hello,

  after skimming through it, I think I completely like what you have
shown here. I'm only concerned about this:

Dear diary, on Mon, Jul 25, 2005 at 12:49:33AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> told me that...
>     $ cat Documentation/.gitignore
>     # ignore generated html files,
>     # except foo.html which is maintained by hand
>     !foo.html
>     *.html

  I think this is wrong, and my brief experiments confirm that. I think
that the actually useful semantics of exclusion would be for
_subsequent_ exclusions, not preliminary ones. You generally don't say
"I never want this ignored, but I want the rest of that ignored", but
"I want that ignored, except this". This also gives you more
flexibility:

	*.html
	!f*.html
	fo*.html

would ignore *.html and fo*.html, but not any other f*.html filenames.
But more importantly,

	.gitignore: *.txt
	Documentation/.gitignore: !*.txt

will not work, which was the whole _point_ of the exclusion.

Could we please have this semantics changed for those reasons?

Thanks,

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
If you want the holes in your knowledge showing up try teaching
someone.  -- Alan Cox

Re: [RFC] extending git-ls-files --exclude.

[Petr Baudis]

Dear diary, on Mon, Jul 25, 2005 at 10:27:36PM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> told me that...
> Linus Torvalds <torvalds@osdl.org> writes:
> 
> > On Mon, 25 Jul 2005, Junio C Hamano wrote:
> >> 
> >> I personally do not have preference either way, but am slightly
> >> biased towards the "cumulative" behaviour the patch attempts to
> >> implement, which was what Pasky said he wanted to have.
> >
> > I think that makes sense.
> >
> > Imagine, for example, that you have separate subdirectory structures for 
> > Documentation and for source - maybe you'd put the "*.o" rule in the 
> > source directory, and a "*.1" rule in the Docs subdirectory.
> 
> I imagined it, but it appears to me that this is a bad example.
> My understanding of what Catalin and the proposed patch
> disagrees is whether the patterns in .gitignore at the top level
> should govern files under ppc/ and mozilla-sha1/ subdirectories;
> Catalin thinks they should not.

They should. If you don't want them take effect in most of the
directories, you would add them as ./pattern in the parent directory,
while if you want them to take effect in most of the directories, you
would exclude them in the ones you don't want the pattern to take effect
in (if you accept my proposal for ! semantics change).

> What I meant by "cumulative" (now I realize I might have
> misunderstood what Pasky wanted to mean by that word, though)
> was not just .gitignore in subdirectory being added, but the
> effect of patterns being added so far, either from the command
> line or by parent directories, last while in the deeper
> directories.

Yes, that's what I meant.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
If you want the holes in your knowledge showing up try teaching
someone.  -- Alan Cox

Re: [RFC] extending git-ls-files --exclude.

[A Large Angry SCM]

Petr Baudis wrote:
>   I think this is wrong, and my brief experiments confirm that. I think
> that the actually useful semantics of exclusion would be for
> _subsequent_ exclusions, not preliminary ones. You generally don't say
> "I never want this ignored, but I want the rest of that ignored", but
> "I want that ignored, except this". This also gives you more
> flexibility:
> 
> 	*.html
> 	!f*.html
> 	fo*.html
> 
> would ignore *.html and fo*.html, but not any other f*.html filenames.
> But more importantly,
> 
> 	.gitignore: *.txt
> 	Documentation/.gitignore: !*.txt
> 
> will not work, which was the whole _point_ of the exclusion.

So you're arguing for "last match wins" versus "first match wins". I, 
personally, find the former more natural and easier to debug by hand.

Re: [RFC] extending git-ls-files --exclude.

[Matthias Urlichs]

Hi, A Large Angry SCM wrote:

> So you're arguing for "last match wins" versus "first match wins". I, 
> personally, find the former more natural and easier to debug by hand.

You know, up until five minutes ago, I thought so too.

However ... as a human being, I liik for meaning, not for processing
instructions. Thus, reading the list top-down, this

> 	*.html
> 	!f*.html
> 	fo*.html

makes perfect sense to me. ("Throw away the HTML files. Well, except for
those that start with 'f'. Well, *except* for foobar.html or whatever.")

The other way round, however, the sequence
> 	fo*.html
> 	!f*.html
> 	*.html

is not immediately understandable in one pass, as the second line makes no
sense whatsoever without the third one. ("Throw away foobar.html. Umm,
we're keeping everything anyway, so why ... oh, HTML files are junked, OK,
so ... now ... umm, what did I want to find out in the first place?")

It gets even more confusing when you're lazy and omit the .html suffix in
the second line.

YMMV, and all that.
-- 
Matthias Urlichs   |   {M:U} IT Design @ m-u-it.de   |  smurf@smurf.noris.de
Disclaimer: The quote was selected randomly. Really. | http://smurf.noris.de
 - -
(It is an old Debian tradition to leave at least twice a year ...)
	-- Sven Rudolph

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Petr Baudis]

Dear diary, on Sat, Jul 23, 2005 at 12:27:31PM CEST, I got a letter
where Catalin Marinas <catalin.marinas@gmail.com> told me that...
> > Agreed. What Cogito uses:
> > 
> > 	.git/author	Default author information in format
> > 				Person Name <email@addy>
> 
> What about .git/committer? This is useful if I do a commit at work and I
> want the repository to have my gmail address.

The committer field generally identifies the committer "physically", and
isn't usually overriden. You'll find <xpasky@machine.sinus.cz> in my
committer field, e.g.

> > 	.git/branch-name
> > 			Symbolic name of the branch of this repository.
> 
> Isn't this the same as $(readlink .git/HEAD), with some trimming?

No, that is something totally separate from the usual concepts of
branches and repositories, designed to make it possible to distinguish
between repositories etc. from the outside of the system.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
If you want the holes in your knowledge showing up try teaching
someone.  -- Alan Cox

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> The committer field generally identifies the committer "physically", and
> isn't usually overriden. You'll find <xpasky@machine.sinus.cz> in my
> committer field, e.g.

I do not want to get involved in policy decisions, but for the
record I always hated your commit log for that "identifies the
committer physically" approach.

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Linus Torvalds]

On Thu, 28 Jul 2005, Junio C Hamano wrote:
> 
> I do not want to get involved in policy decisions, but for the
> record I always hated your commit log for that "identifies the
> committer physically" approach.

Well, I have to say that I find it quite useful myself. I try to commit
x86 patches to the kernel on an x86 machine (I had better had tested them
there), so they usually say "torvalds@evo.osdl.org" or "torvalds@yonah..",
while my normal patches tend to say "torvalds@g5.." since that's my main
machine.

So I tend to not override it, even though none of those are really valid
email addresses.

		Linus

Re: [RFC] extending git-ls-files --exclude.

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> You generally don't say "I never want this ignored, but I want
> the rest of that ignored", but "I want that ignored, except
> this".

OK.

> But more importantly,
>
> 	.gitignore: *.txt
> 	Documentation/.gitignore: !*.txt
>
> will not work, which was the whole _point_ of the exclusion.

I agree that this is a bigger issue.

My updated proposal is as follows:

 * We collect --exclude patterns in "command line patterns"
   list, in the same order as given on the command line.

 * We collect patterns read from the files specified with
   --exclude-from in "exclude-from patterns" list, in the same
   order as given on the command line and the same order
   patterns appear in the file.

 * While we descend the directories, files named by EPD flag, if
   found, are read from top to bottom and concatenated into the
   "per directory patterns" list.  When leaving the directory,
   the patterns read from that directory's EPD file are popped
   off.

 * When checking a file to see if it is excluded, we first look
   at "exclude-from patterns" list, then "per directory
   patterns" list, and then "command line patterns list", in
   that order.  The last match wins [*1*].

An example:

You have three files in your git source tree and your $HOME:

    git/.gitignore                      lists patterns A and B
    git/Documentation/.gitignore        lists patterns C and D
    git/.git/info/ignore                lists patterns E and F
    $HOME/.gitrc/ignore                 lists patterns G and H

You say:

    git-ls-files --others \
        --exclude=I --exclude=J \
        --exclude-from=.git/info/ignore \
        --exclude-from=~/.gitrc/ignore \
        --exclude-per-directory=.gitignore \

While in git/ directory itself, the following patterns are
checked and the last match wins:

    E F G H   A B       I J

When we descend into git/Documentation, the list of patterns
used becomes the following, still the last match wins:

    E F G H   A B C D   I J

The reason --exclude-from comes first (i.e. having the least say
in the outcome) and --exclude comes last (i.e. having the most
say) is because the former is to give the overall fallback
default, and the latter is the ultimate end user preference.

Does this sound reasonable?

[Footnote]

*1* In real implementation, I would probably scan in reverse
from the end and stop at the first match, but that is an
implementation detail.

Re: [RFC] extending git-ls-files --exclude.

[Petr Baudis]

Dear diary, on Thu, Jul 28, 2005 at 09:25:45PM CEST, I got a letter
where Matthias Urlichs <smurf@smurf.noris.de> told me that...
> Hi, A Large Angry SCM wrote:
> 
> > So you're arguing for "last match wins" versus "first match wins". I, 
> > personally, find the former more natural and easier to debug by hand.
> 
> You know, up until five minutes ago, I thought so too.

So is the Large Angry SCM agreeing with me or not? I wrote long reply to
his mail, then reread what he wrote again, and decided that he is
_agreeing_ with me and you that "last match wins" is better. :-)

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
If you want the holes in your knowledge showing up try teaching
someone.  -- Alan Cox

Re: [RFC] extending git-ls-files --exclude.

[Petr Baudis]

Dear diary, on Fri, Jul 29, 2005 at 07:04:36AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> told me that...
>  * When checking a file to see if it is excluded, we first look
>    at "exclude-from patterns" list, then "per directory
>    patterns" list, and then "command line patterns list", in
>    that order.  The last match wins [*1*].

Hmm. What about just excluding the files according to the order of
parameters on the command line?

Here, the question is whether the GIT Core tools should provide full
flexibility and friendness to custom use, or rather serve as tighter
unifying layer for the porcelains, enforcing certain conventions. That's
up to you to decide, obviously, but perhaps someone will want to use the
exclude mechanisms for something else than the "classic" other files
ignoring stuff, and generally more flexibility might be better. So I'd
argue for codifying those conventions at the level of the porcelain
users and not enforcing them in git-ls-files itself.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
If you want the holes in your knowledge showing up try teaching
someone.  -- Alan Cox

Re: [RFC] extending git-ls-files --exclude.

[Matthias Urlichs]

Hi,

Petr Baudis:
> Dear diary, on Thu, Jul 28, 2005 at 09:25:45PM CEST, I got a letter
> where Matthias Urlichs <smurf@smurf.noris.de> told me that...
> > Hi, A Large Angry SCM wrote:
> > 
> > > So you're arguing for "last match wins" versus "first match wins". I, 
> > > personally, find the former more natural and easier to debug by hand.
> > 
> > You know, up until five minutes ago, I thought so too.
> 
> So is the Large Angry SCM agreeing with me or not? I wrote long reply to
> his mail, then reread what he wrote again, and decided that he is
> _agreeing_ with me and you that "last match wins" is better. :-)
> 
Bah, I misparsed his sentence (I read "former" as "first wins"), otherwise
my reply would have been worded slightly differently.

-- 
Matthias Urlichs   |   {M:U} IT Design @ m-u-it.de   |  smurf@smurf.noris.de
Disclaimer: The quote was selected randomly. Really. | http://smurf.noris.de
 - -
"All these black people are screwing up my democracy." - Ian Smith

[PATCH] ls-files: rework exclude patterns.

[Junio C Hamano]

Pasky and others raised many valid points on the problems
initial exclude pattern enhancement work had.  Based on the
list discussion, rework the exclude logic to use "last match
determines its fate" rule, and order the list by exclude-from
(the fallback default pattern file), exclude-per-directory
(shallower to deeper, so deeper ones can override), and then
command line exclude patterns.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

 ls-files.c |  100 +++++++++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 72 insertions(+), 28 deletions(-)

a908ed1b0fed52bfdcfc8b3ada366ce05e44c887
diff --git a/ls-files.c b/ls-files.c
--- a/ls-files.c
+++ b/ls-files.c
@@ -26,30 +26,45 @@ static const char *tag_other = "";
 static const char *tag_killed = "";
 
 static char *exclude_per_dir = NULL;
-static int nr_excludes;
-static int excludes_alloc;
-static struct exclude {
-	const char *pattern;
-	const char *base;
-	int baselen;
-} **excludes;
 
-static void add_exclude(const char *string, const char *base, int baselen)
+/* We maintain three exclude pattern lists:
+ * EXC_CMDL lists patterns explicitly given on the command line.
+ * EXC_DIRS lists patterns obtained from per-directory ignore files.
+ * EXC_FILE lists patterns from fallback ignore files.
+ */
+#define EXC_CMDL 0
+#define EXC_DIRS 1
+#define EXC_FILE 2
+static struct exclude_list {
+	int nr;
+	int alloc;
+	struct exclude {
+		const char *pattern;
+		const char *base;
+		int baselen;
+	} **excludes;
+} exclude_list[3];
+
+static void add_exclude(const char *string, const char *base,
+			int baselen, struct exclude_list *which)
 {
 	struct exclude *x = xmalloc(sizeof (*x));
 
 	x->pattern = string;
 	x->base = base;
 	x->baselen = baselen;
-	if (nr_excludes == excludes_alloc) {
-		excludes_alloc = alloc_nr(excludes_alloc);
-		excludes = realloc(excludes, excludes_alloc*sizeof(char *));
+	if (which->nr == which->alloc) {
+		which->alloc = alloc_nr(which->alloc);
+		which->excludes = realloc(which->excludes,
+					  which->alloc * sizeof(x));
 	}
-	excludes[nr_excludes++] = x;
+	which->excludes[which->nr++] = x;
 }
 
 static int add_excludes_from_file_1(const char *fname,
-				    const char *base, int baselen)
+				    const char *base,
+				    int baselen,
+				    struct exclude_list *which)
 {
 	int fd, i;
 	long size;
@@ -76,7 +91,7 @@ static int add_excludes_from_file_1(cons
 		if (buf[i] == '\n') {
 			if (entry != buf + i && entry[0] != '#') {
 				buf[i] = 0;
-				add_exclude(entry, base, baselen);
+				add_exclude(entry, base, baselen, which);
 			}
 			entry = buf + i + 1;
 		}
@@ -91,38 +106,45 @@ static int add_excludes_from_file_1(cons
 
 static void add_excludes_from_file(const char *fname)
 {
-	if (add_excludes_from_file_1(fname, "", 0) < 0)
+	if (add_excludes_from_file_1(fname, "", 0,
+				     &exclude_list[EXC_FILE]) < 0)
 		die("cannot use %s as an exclude file", fname);
 }
 
 static int push_exclude_per_directory(const char *base, int baselen)
 {
 	char exclude_file[PATH_MAX];
-	int current_nr = nr_excludes;
+	struct exclude_list *el = &exclude_list[EXC_DIRS];
+	int current_nr = el->nr;
 
 	if (exclude_per_dir) {
 		memcpy(exclude_file, base, baselen);
 		strcpy(exclude_file + baselen, exclude_per_dir);
-		add_excludes_from_file_1(exclude_file, base, baselen);
+		add_excludes_from_file_1(exclude_file, base, baselen, el);
 	}
 	return current_nr;
 }
 
 static void pop_exclude_per_directory(int stk)
 {
-	while (stk < nr_excludes)
-		free(excludes[--nr_excludes]);
+	struct exclude_list *el = &exclude_list[EXC_DIRS];
+
+	while (stk < el->nr)
+		free(el->excludes[--el->nr]);
 }
 
-static int excluded(const char *pathname)
+/* Scan the list and let the last match determines the fate.
+ * Return 1 for exclude, 0 for include and -1 for undecided.
+ */
+static int excluded_1(const char *pathname,
+		      int pathlen,
+		      struct exclude_list *el)
 {
 	int i;
 
-	if (nr_excludes) {
-		int pathlen = strlen(pathname);
-
-		for (i = 0; i < nr_excludes; i++) {
-			struct exclude *x = excludes[i];
+	if (el->nr) {
+		for (i = el->nr - 1; 0 <= i; i--) {
+			struct exclude *x = el->excludes[i];
 			const char *exclude = x->pattern;
 			int to_exclude = 1;
 
@@ -158,6 +180,22 @@ static int excluded(const char *pathname
 			}
 		}
 	}
+	return -1; /* undecided */
+}
+
+static int excluded(const char *pathname)
+{
+	int pathlen = strlen(pathname);
+	int st;
+
+	for (st = EXC_CMDL; st <= EXC_FILE; st++) {
+		switch (excluded_1(pathname, pathlen, &exclude_list[st])) {
+		case 0:
+			return 0;
+		case 1:
+			return 1;
+		}
+	}
 	return 0;
 }
 
@@ -371,6 +409,7 @@ static const char *ls_files_usage =
 int main(int argc, char **argv)
 {
 	int i;
+	int exc_given = 0;
 
 	for (i = 1; i < argc; i++) {
 		char *arg = argv[i];
@@ -402,20 +441,25 @@ int main(int argc, char **argv)
 			show_stage = 1;
 			show_unmerged = 1;
 		} else if (!strcmp(arg, "-x") && i+1 < argc) {
-			add_exclude(argv[++i], "", 0);
+			exc_given = 1;
+			add_exclude(argv[++i], "", 0, &exclude_list[EXC_CMDL]);
 		} else if (!strncmp(arg, "--exclude=", 10)) {
-			add_exclude(arg+10, "", 0);
+			exc_given = 1;
+			add_exclude(arg+10, "", 0, &exclude_list[EXC_CMDL]);
 		} else if (!strcmp(arg, "-X") && i+1 < argc) {
+			exc_given = 1;
 			add_excludes_from_file(argv[++i]);
 		} else if (!strncmp(arg, "--exclude-from=", 15)) {
+			exc_given = 1;
 			add_excludes_from_file(arg+15);
 		} else if (!strncmp(arg, "--exclude-per-directory=", 24)) {
+			exc_given = 1;
 			exclude_per_dir = arg + 24;
 		} else
 			usage(ls_files_usage);
 	}
 
-	if (show_ignored && !nr_excludes) {
+	if (show_ignored && !exc_given) {
 		fprintf(stderr, "%s: --ignored needs some exclude pattern\n",
 			argv[0]);
 		exit(1);

[PATCH] Documentation and tests: ls-files exclude pattern.

[Junio C Hamano]

Update the tests and documentation to match the new "last one
determines its fate" semantics.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

 Documentation/git-ls-files.txt     |   27 ++++++++++++++-------------
 t/t3001-ls-files-others-exclude.sh |   23 ++++++++++++++++++-----
 2 files changed, 32 insertions(+), 18 deletions(-)

4ae2315518ee4017c6646a4421cf448297fc6dcc
diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
--- a/Documentation/git-ls-files.txt
+++ b/Documentation/git-ls-files.txt
@@ -115,14 +115,14 @@ An exclude pattern file used by (2) and 
 per line.  A line that starts with a '#' can be used as comment
 for readability.
 
-The list of patterns that is in effect at a given time is
-built and ordered in the following way:
+There are three lists of patterns that are in effect at a given
+time.  They are built and ordered in the following way:
 
- * --exclude=<pattern> and lines read from --exclude-from=<file>
-   come at the beginning of the list of patterns, in the order
-   given on the command line.  Patterns that come from the file
-   specified with --exclude-from are ordered in the same order
-   as they appear in the file.
+ * --exclude=<pattern> from the command line; patterns are
+   ordered in the same order as they appear on the command line.
+
+ * lines read from --exclude-from=<file>; patterns are ordered
+   in the same order as they appear in the file.
 
  * When --exclude-per-directory=<name> is specified, upon
    entering a directory that has such a file, its contents are
@@ -130,11 +130,12 @@ built and ordered in the following way:
    are popped off when leaving the directory.
 
 Each pattern in the pattern list specifies "a match pattern" and
-optionally the fate --- either a file that matches the pattern
-is considered excluded or included.  By default, this being
-"exclude" mechanism, the fate is "excluded".  A filename is
-examined against the patterns in the list, and the first match
-determines its fate.
+optionally the fate;n either a file that matches the pattern is
+considered excluded or included.  A filename is matched against
+the patterns in the three lists; the --exclude-from list is
+checked first, then the --exclude-per-directory list, and then
+finally the --exclude list. The last match determines its fate.
+If there is no match in the three lists, the fate is "included".
 
 A pattern specified on the command line with --exclude or read
 from the file specified with --exclude-from is relative to the
@@ -168,9 +169,9 @@ An example:
     *.[oa]
     $ cat Documentation/.gitignore
     # ignore generated html files,
+    *.html
     # except foo.html which is maintained by hand
     !foo.html
-    *.html
     $ git-ls-files --ignored \
         --exclude='Documentation/*.[0-9]' \
         --exclude-from=.git/ignore \
diff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh
--- a/t/t3001-ls-files-others-exclude.sh
+++ b/t/t3001-ls-files-others-exclude.sh
@@ -14,7 +14,7 @@ rm -fr one three
 for dir in . one one/two three
 do
   mkdir -p $dir &&
-  for i in 1 2 3 4 5
+  for i in 1 2 3 4 5 6 7 8
   do
     >$dir/a.$i
   done
@@ -24,31 +24,44 @@ cat >expect <<EOF
 a.2
 a.4
 a.5
+a.8
 one/a.3
 one/a.4
 one/a.5
+one/a.7
+one/two/a.2
 one/two/a.3
 one/two/a.5
+one/two/a.7
+one/two/a.8
 three/a.2
 three/a.3
 three/a.4
 three/a.5
+three/a.8
 EOF
 
 echo '.gitignore
 output
 expect
 .gitignore
-' >.git/ignore
+*.7
+!*.8' >.git/ignore
 
 echo '*.1
-/*.3' >.gitignore
+/*.3
+!*.6' >.gitignore
 echo '*.2
-two/*.4' >one/.gitignore
+two/*.4
+!*.7
+*.8' >one/.gitignore
+echo '!*.2
+!*.8' >one/two/.gitignore
 
 test_expect_success \
-    'git-ls-files --others --exclude.' \
+    'git-ls-files --others with various exclude options.' \
     'git-ls-files --others \
+       --exclude=\*.6 \
        --exclude-per-directory=.gitignore \
        --exclude-from=.git/ignore \
        >output &&

Re: [RFC] extending git-ls-files --exclude.

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> Hmm. What about just excluding the files according to the order of
> parameters on the command line?
>
> Here, the question is whether the GIT Core tools should provide full
> flexibility and friendness to custom use, or rather serve as tighter
> unifying layer for the porcelains, enforcing certain conventions.

While I would in principle prefer to offer more freedom to shoot
yourselves in the foot ;-), the pragmatic side of me says too
much flexibility is just asking for trouble with not much
additional gain.  For example, your "generic first, and then
list exceptions" argument convinced me to shelve the "first
match wins" rule, but I _could_ have added an extra option to
allow other Porcelain writers who want to have "most number of
match wins" rule while at it.  I didn't.  Let's wait and see if
somebody else comes up with a different use scenario that would
be useful in real life.

In the meantime, the current one is clearly broken as you
pointed out, so let's replace it with the updated "generic rule
with the following exceptions" one.

Re: [RFC] extending git-ls-files --exclude.

[Petr Baudis]

Dear diary, on Fri, Jul 29, 2005 at 10:24:54AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> told me that...
> In the meantime, the current one is clearly broken as you
> pointed out, so let's replace it with the updated "generic rule
> with the following exceptions" one.

That's fine by me. I would only like to ask the Porcelain authors to
keep their git-ls-files exclude parameters order matching the internal
ordering so that when we indeed change it in the future to follow the
commandline order, the Porcelains won't break.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
If you want the holes in your knowledge showing up try teaching
someone.  -- Alan Cox

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Catalin Marinas]

Petr Baudis <pasky@suse.cz> wrote:
> The committer field generally identifies the committer "physically", and
> isn't usually overriden. You'll find <xpasky@machine.sinus.cz> in my
> committer field, e.g.

I thought GIT_COMMITTER_{NAME,EMAIL} were added to be able to override
the defaults like username@localmachine.

The latest StGIT snapshot uses, by default, the committer's details
for the From: line when sending patches by e-mail, assuming that this
is a valid e-mail address. One can define his own e-mail template and
use a different From: line but I thought it would be simpler to
provide some defaults based on this.

If StGIT users don't like this default template, I can change it.

-- 
Catalin

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Petr Baudis]

Dear diary, on Fri, Jul 29, 2005 at 11:55:52AM CEST, I got a letter
where Catalin Marinas <catalin.marinas@gmail.com> told me that...
> Petr Baudis <pasky@suse.cz> wrote:
> > The committer field generally identifies the committer "physically", and
> > isn't usually overriden. You'll find <xpasky@machine.sinus.cz> in my
> > committer field, e.g.
> 
> I thought GIT_COMMITTER_{NAME,EMAIL} were added to be able to override
> the defaults like username@localmachine.

Yes, but IIRC only for rather special cases like recommitting older
commits, importing from other VCSes, etc.

> The latest StGIT snapshot uses, by default, the committer's details
> for the From: line when sending patches by e-mail, assuming that this
> is a valid e-mail address. One can define his own e-mail template and
> use a different From: line but I thought it would be simpler to
> provide some defaults based on this.

Why don't you rather use the GIT_AUTHOR_* variables?

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
If you want the holes in your knowledge showing up try teaching
someone.  -- Alan Cox

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Catalin Marinas]

Petr Baudis <pasky@suse.cz> wrote:
> Dear diary, on Fri, Jul 29, 2005 at 11:55:52AM CEST, I got a letter
> where Catalin Marinas <catalin.marinas@gmail.com> told me that...
>> The latest StGIT snapshot uses, by default, the committer's details
>> for the From: line when sending patches by e-mail, assuming that this
>> is a valid e-mail address. One can define his own e-mail template and
>> use a different From: line but I thought it would be simpler to
>> provide some defaults based on this.
>
> Why don't you rather use the GIT_AUTHOR_* variables?

It was simpler since the template variables were based on the patch
details. Anyway, it's not hard to modify.

-- 
Catalin

Re: [RFC] extending git-ls-files --exclude.

[A Large Angry SCM]

Petr Baudis wrote:
> Dear diary, on Thu, Jul 28, 2005 at 09:25:45PM CEST, I got a letter
> where Matthias Urlichs <smurf@smurf.noris.de> told me that...
>>Hi, A Large Angry SCM wrote:
>>
>>>So you're arguing for "last match wins" versus "first match wins". I, 
>>>personally, find the former more natural and easier to debug by hand.
>>You know, up until five minutes ago, I thought so too.
> 
> So is the Large Angry SCM agreeing with me or not? I wrote long reply to
> his mail, then reread what he wrote again, and decided that he is
> _agreeing_ with me and you that "last match wins" is better. :-)
> 

*Oops!*

Yes, it looks that way doesn't it. But I had accidentally [*1*] typed 
"former" where I wanted "later". Either way works as long as it's well 
documented/understood.

[*1*] Either too much or too little caffeine at the time I suspect.

Re: [PATCH 1/1] Tell vim the textwidth is 75.

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> Dear diary, on Fri, Jul 29, 2005 at 11:55:52AM CEST, I got a letter
> where Catalin Marinas <catalin.marinas@gmail.com> told me that...
>> Petr Baudis <pasky@suse.cz> wrote:
>> > The committer field generally identifies the committer "physically", and
>> > isn't usually overriden. You'll find <xpasky@machine.sinus.cz> in my
>> > committer field, e.g.
>> 
>> I thought GIT_COMMITTER_{NAME,EMAIL} were added to be able to override
>> the defaults like username@localmachine.
>
> Yes, but IIRC only for rather special cases like recommitting older
> commits, importing from other VCSes, etc.

My recollection coincides with Catalin.  Special case is when
you set it to somebody else, "logically" different from you, as
you say.  Fixing GECOS fields and mailhost settings as Catalin
says is another.

Setting it to something "physically" different but still the
same you, for example using your work e-mail address when you
are on your home machine, is not special case at all, but is
also a valid use.  However when you start to do that, it may
become unusable as a valid e-mail address you can use for From:
and/or Sender: fields.

>> The latest StGIT snapshot uses, by default, the committer's details
>> for the From: line when sending patches by e-mail, assuming that this
>> is a valid e-mail address. One can define his own e-mail template and
>> use a different From: line but I thought it would be simpler to
>> provide some defaults based on this.
>
> Why don't you rather use the GIT_AUTHOR_* variables?

I tend to agree.  When forwarding somebody else's commit as a
patch, you would want to name that author on From: line and make
yourself Sender:.  Of course, in that case Sender: should be a
valid e-mail address for the originating machine; otherwise
bogus-mail-relay prevention would block your e-mail.  In that
sense, using COMMITTER to identify you "physically" (meaning:
tied to your identity with that particular machine) would work
better.

Re: [RFC] extending git-ls-files --exclude.

[Wayne Scott]

On 7/29/05, Junio C Hamano <junkio@cox.net> wrote:
> While I would in principle prefer to offer more freedom to shoot
> yourselves in the foot ;-), the pragmatic side of me says too
> much flexibility is just asking for trouble with not much
> additional gain.  

For an example of just how far you can go down the road to mind
numbing complexity try reading the rsync manpage about how to exclude
files.

-Wayne