[PATCH] Support ent:relative_path

[PATCH] Support ent:relative_path

[Dana How]

Most commands accept relative paths,  but this is
not true of arguments in ent:path format.  This
patch makes all 3 of the following git-show commands
work in the git source tree (not just the first):
 % cd xdiff
 % git-show v1.5.2-rc0:xdiff/xemit.h
 % git-show v1.5.2-rc0:./xemit.h
 % git-config --bool core.relativepaths yes
 % git-show v1.5.2-rc0:xemit.h

Signed-off-by: Dana L. How <danahow@gmail.com>
---
 cache.h       |    2 ++
 config.c      |    5 +++++
 environment.c |    1 +
 setup.c       |    5 ++++-
 sha1_name.c   |   27 ++++++++++++++++++++++++---
 5 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/cache.h b/cache.h
index 8e76152..fc3fcb1 100644
--- a/cache.h
+++ b/cache.h
@@ -215,6 +215,7 @@ extern char *get_graft_file(void);
 
 #define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
 
+extern const char *prefix_to_cwd;
 extern const char **get_pathspec(const char *prefix, const char **pathspec);
 extern const char *setup_git_directory_gently(int *);
 extern const char *setup_git_directory(void);
@@ -276,6 +277,7 @@ extern int delete_ref(const char *, const unsigned char *sha1);
 extern int use_legacy_headers;
 extern int trust_executable_bit;
 extern int has_symlinks;
+extern int assume_relative_paths;
 extern int assume_unchanged;
 extern int prefer_symlink_refs;
 extern int log_all_ref_updates;
diff --git a/config.c b/config.c
index 70d1055..7525965 100644
--- a/config.c
+++ b/config.c
@@ -279,6 +279,11 @@ int git_default_config(const char *var, const char *value)
 		return 0;
 	}
 
+	if (!strcmp(var, "core.relativepaths")) {
+		assume_relative_paths = git_config_bool(var, value);
+		return 0;
+	}
+
 	if (!strcmp(var, "core.ignorestat")) {
 		assume_unchanged = git_config_bool(var, value);
 		return 0;
diff --git a/environment.c b/environment.c
index 2231659..f1b867d 100644
--- a/environment.c
+++ b/environment.c
@@ -14,6 +14,7 @@ char git_default_name[MAX_GITNAME];
 int use_legacy_headers = 1;
 int trust_executable_bit = 1;
 int has_symlinks = 1;
+int assume_relative_paths;
 int assume_unchanged;
 int prefer_symlink_refs;
 int is_bare_repository_cfg = -1; /* unspecified */
diff --git a/setup.c b/setup.c
index a45ea83..46ae6e3 100644
--- a/setup.c
+++ b/setup.c
@@ -1,5 +1,7 @@
 #include "cache.h"
 
+const char *prefix_to_cwd;
+
 const char *prefix_path(const char *prefix, int len, const char *path)
 {
 	const char *orig = path;
@@ -252,7 +254,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	cwd[len++] = '/';
 	cwd[len] = 0;
 	inside_git_dir = !prefixcmp(cwd + offset, ".git/");
-	return cwd + offset;
+	prefix_to_cwd = cwd + offset;
+	return prefix_to_cwd;
 }
 
 int git_config_perm(const char *var, const char *value)
diff --git a/sha1_name.c b/sha1_name.c
index 55f25a2..0b9e92c 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -592,6 +592,24 @@ static int handle_one_ref(const char *path,
 	return 0;
 }
 
+static void prepend_prefix(const char **cp, int *namelen)
+{
+	static char fullpath[PATH_MAX];
+	if (*namelen > 2 && !memcmp(*cp, "./", 2)) {
+		*cp += 2;
+		*namelen -= 2;
+	} else
+	if (!assume_relative_paths || !prefix_to_cwd)
+		return;
+
+	*namelen += strlen(prefix_to_cwd);
+	if (*namelen >= PATH_MAX)
+		die("path too long");
+	strcpy(fullpath, prefix_to_cwd);
+	strcat(fullpath, *cp);
+	*cp = fullpath;
+}
+
 /*
  * This interprets names like ':/Initial revision of "git"' by searching
  * through history and returning the first commit whose message starts
@@ -681,6 +699,7 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
 			read_cache();
 		if (active_nr < 0)
 			return -1;
+		prepend_prefix(&cp, &namelen);
 		pos = cache_name_pos(cp, namelen);
 		if (pos < 0)
 			pos = -pos - 1;
@@ -708,9 +727,11 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
 	}
 	if (*cp == ':') {
 		unsigned char tree_sha1[20];
-		if (!get_sha1_1(name, cp-name, tree_sha1))
-			return get_tree_entry(tree_sha1, cp+1, sha1,
-					      mode);
+		if (!get_sha1_1(name, cp - name, tree_sha1)) {
+			namelen -= ++cp - name;
+			prepend_prefix(&cp, &namelen);
+			return get_tree_entry(tree_sha1, cp, sha1, mode);
+		}
 	}
 	return ret;
 }
-- 
1.5.2.rc0.787.g0014

Re: [PATCH] Support ent:relative_path

[Dana How]

Yikes... the if's in prepend_prefix() need to be re-arranged.

Dana

On 5/4/07, Dana How <danahow@gmail.com> wrote:
>
> Most commands accept relative paths,  but this is
> not true of arguments in ent:path format.  This
> patch makes all 3 of the following git-show commands
> work in the git source tree (not just the first):
>  % cd xdiff
>  % git-show v1.5.2-rc0:xdiff/xemit.h
>  % git-show v1.5.2-rc0:./xemit.h
>  % git-config --bool core.relativepaths yes
>  % git-show v1.5.2-rc0:xemit.h
>
> Signed-off-by: Dana L. How <danahow@gmail.com>
> ---
>  cache.h       |    2 ++
>  config.c      |    5 +++++
>  environment.c |    1 +
>  setup.c       |    5 ++++-
>  sha1_name.c   |   27 ++++++++++++++++++++++++---
>  5 files changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index 8e76152..fc3fcb1 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -215,6 +215,7 @@ extern char *get_graft_file(void);
>
>  #define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
>
> +extern const char *prefix_to_cwd;
>  extern const char **get_pathspec(const char *prefix, const char **pathspec);
>  extern const char *setup_git_directory_gently(int *);
>  extern const char *setup_git_directory(void);
> @@ -276,6 +277,7 @@ extern int delete_ref(const char *, const unsigned char *sha1);
>  extern int use_legacy_headers;
>  extern int trust_executable_bit;
>  extern int has_symlinks;
> +extern int assume_relative_paths;
>  extern int assume_unchanged;
>  extern int prefer_symlink_refs;
>  extern int log_all_ref_updates;
> diff --git a/config.c b/config.c
> index 70d1055..7525965 100644
> --- a/config.c
> +++ b/config.c
> @@ -279,6 +279,11 @@ int git_default_config(const char *var, const char *value)
>                 return 0;
>         }
>
> +       if (!strcmp(var, "core.relativepaths")) {
> +               assume_relative_paths = git_config_bool(var, value);
> +               return 0;
> +       }
> +
>         if (!strcmp(var, "core.ignorestat")) {
>                 assume_unchanged = git_config_bool(var, value);
>                 return 0;
> diff --git a/environment.c b/environment.c
> index 2231659..f1b867d 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -14,6 +14,7 @@ char git_default_name[MAX_GITNAME];
>  int use_legacy_headers = 1;
>  int trust_executable_bit = 1;
>  int has_symlinks = 1;
> +int assume_relative_paths;
>  int assume_unchanged;
>  int prefer_symlink_refs;
>  int is_bare_repository_cfg = -1; /* unspecified */
> diff --git a/setup.c b/setup.c
> index a45ea83..46ae6e3 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -1,5 +1,7 @@
>  #include "cache.h"
>
> +const char *prefix_to_cwd;
> +
>  const char *prefix_path(const char *prefix, int len, const char *path)
>  {
>         const char *orig = path;
> @@ -252,7 +254,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
>         cwd[len++] = '/';
>         cwd[len] = 0;
>         inside_git_dir = !prefixcmp(cwd + offset, ".git/");
> -       return cwd + offset;
> +       prefix_to_cwd = cwd + offset;
> +       return prefix_to_cwd;
>  }
>
>  int git_config_perm(const char *var, const char *value)
> diff --git a/sha1_name.c b/sha1_name.c
> index 55f25a2..0b9e92c 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -592,6 +592,24 @@ static int handle_one_ref(const char *path,
>         return 0;
>  }
>
> +static void prepend_prefix(const char **cp, int *namelen)
> +{
> +       static char fullpath[PATH_MAX];
> +       if (*namelen > 2 && !memcmp(*cp, "./", 2)) {
> +               *cp += 2;
> +               *namelen -= 2;
> +       } else
> +       if (!assume_relative_paths || !prefix_to_cwd)
> +               return;
> +
> +       *namelen += strlen(prefix_to_cwd);
> +       if (*namelen >= PATH_MAX)
> +               die("path too long");
> +       strcpy(fullpath, prefix_to_cwd);
> +       strcat(fullpath, *cp);
> +       *cp = fullpath;
> +}
> +
>  /*
>   * This interprets names like ':/Initial revision of "git"' by searching
>   * through history and returning the first commit whose message starts
> @@ -681,6 +699,7 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
>                         read_cache();
>                 if (active_nr < 0)
>                         return -1;
> +               prepend_prefix(&cp, &namelen);
>                 pos = cache_name_pos(cp, namelen);
>                 if (pos < 0)
>                         pos = -pos - 1;
> @@ -708,9 +727,11 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
>         }
>         if (*cp == ':') {
>                 unsigned char tree_sha1[20];
> -               if (!get_sha1_1(name, cp-name, tree_sha1))
> -                       return get_tree_entry(tree_sha1, cp+1, sha1,
> -                                             mode);
> +               if (!get_sha1_1(name, cp - name, tree_sha1)) {
> +                       namelen -= ++cp - name;
> +                       prepend_prefix(&cp, &namelen);
> +                       return get_tree_entry(tree_sha1, cp, sha1, mode);
> +               }
>         }
>         return ret;
>  }
> --
> 1.5.2.rc0.787.g0014
>
>


-- 
Dana L. How  danahow@gmail.com  +1 650 804 5991 cell

Re: [PATCH] Support ent:relative_path

[Junio C Hamano]

I do not think it is wrong per-se, to want to make this hold true:

	A=$(git rev-parse :somedir/file)
	B=$(cd somedir && git rev-parse :file)
        test "$A" = "$B"

One thing I am reasonably certain however is that this should
NOT be conditional to a config setting.  Doing so would force
scripts that take (or compute) path and commit and concatenate
them to make "${commit}:${path}" to name a blob (or tree) to
first inspect the current setting of core.relativepaths and undo
what the new code does by prefixing/subtracting the prefix
string depending on the config.

In other words, having that config is not really helping scripts
or compatibility.

I think the choices are:

 (1) we say it was a mistake that we did not make it relative to
     the current directory when we introduced the X:<path>
     syntax (X could be empty or :[0-3]: for index, or a commit
     or tree object name), and change the semantics in a future
     major release for everybody, apologizing for potentially
     breaking existing scripts; or

 (2) keep the current behaviour as is, and come up with a
     different syntax to use relative; or

 (3) do nothing.

My preference is (2), (3) and then (1), but I do not have
offhand a suggestion for a good metacharacter we could use.

Re: [PATCH] Support ent:relative_path

[Dana How]

On 5/4/07, Junio C Hamano <junkio@cox.net> wrote:
> I do not think it is wrong per-se, to want to make this hold true:
>
>         A=$(git rev-parse :somedir/file)
>         B=$(cd somedir && git rev-parse :file)
>         test "$A" = "$B"
>
> One thing I am reasonably certain however is that this should
> NOT be conditional to a config setting.  Doing so would force
> scripts that take (or compute) path and commit and concatenate
> them to make "${commit}:${path}" to name a blob (or tree) to
> first inspect the current setting of core.relativepaths and undo
> what the new code does by prefixing/subtracting the prefix
> string depending on the config.
>
> In other words, having that config is not really helping scripts
> or compatibility.
>
> I think the choices are:
>
>  (1) we say it was a mistake that we did not make it relative to
>      the current directory when we introduced the X:<path>
>      syntax (X could be empty or :[0-3]: for index, or a commit
>      or tree object name), and change the semantics in a future
>      major release for everybody, apologizing for potentially
>      breaking existing scripts; or
>
>  (2) keep the current behaviour as is, and come up with a
>      different syntax to use relative; or
>
>  (3) do nothing.
>
> My preference is (2), (3) and then (1), but I do not have
> offhand a suggestion for a good metacharacter we could use.

Let us require:
* No config settings as you prefer
* Current interpretations are unchanged.

The current path supports
  :fullpath
  :/string
  :./relpath
[assuming core.relativepaths=no always since it goes away].
(:/string is the "find string in commit msg" feature.)

A new metacharacter lets us drop the "./" ,
and I think that's desirable.
How about = ? It's not even shifted on my keyboard:
 ent=relpath
 ent=2=relpath
 ent=/fullpath

1. We would accept the same new metacharacter before stage if any.
2, You could still use ent:/string , or ent:fullpath .

The patch should also be extended so relpath can start
with a sequence of ./ or ../ prefixes,
which is easily handled in prepend_prefix()..

What do you think of "=" ?
-- 
Dana L. How  danahow@gmail.com  +1 650 804 5991 cell

Re: [PATCH] Support ent:relative_path

[Alex Riesen]

On 5/4/07, Junio C Hamano <junkio@cox.net> wrote:
>  (1) we say it was a mistake that we did not make it relative to
>      the current directory when we introduced the X:<path>
>      syntax (X could be empty or :[0-3]: for index, or a commit
>      or tree object name), and change the semantics in a future
>      major release for everybody, apologizing for potentially
>      breaking existing scripts; or

That would be my first prio preference

>  (2) keep the current behaviour as is, and come up with a
>      different syntax to use relative; or
>
>  (3) do nothing.
>
> My preference is (2), (3) and then (1), but I do not have
> offhand a suggestion for a good metacharacter we could use.

"./" :)

Re: [PATCH] Support ent:relative_path

[Dana How]

On 5/4/07, Alex Riesen <raa.lkml@gmail.com> wrote:
> On 5/4/07, Junio C Hamano <junkio@cox.net> wrote:
> >  (1) we say it was a mistake that we did not make it relative to
> >      the current directory when we introduced the X:<path>
> >      syntax (X could be empty or :[0-3]: for index, or a commit
> >      or tree object name), and change the semantics in a future
> >      major release for everybody, apologizing for potentially
> >      breaking existing scripts; or
>
> That would be my first prio preference
Mine too,  but I don't want to break anything.

> >  (2) keep the current behaviour as is, and come up with a
> >      different syntax to use relative; or
> >
> >  (3) do nothing.
> >
> > My preference is (2), (3) and then (1), but I do not have
> > offhand a suggestion for a good metacharacter we could use.
>
> "./" :)
Heh.  Yes, that works in the current patch.

I'm really starting to like the idea of introducing "="
as an alternative for ":" :
"=" assumes relative paths, ":" assumes absolute (full) paths.
That could be Junio's new metacharacter.

Since "=" is slightly easier to type [ ;-) ], and looks like a stretched ":",
it's almost ingenious...
(Definitely getting carried away!)
-- 
Dana L. How  danahow@gmail.com  +1 650 804 5991 cell

Re: [PATCH] Support ent:relative_path

[Alex Riesen]

On 5/4/07, Dana How <danahow@gmail.com> wrote:
> > >  (1) we say it was a mistake that we did not make it relative to
> > >      the current directory when we introduced the X:<path>
> > >      syntax (X could be empty or :[0-3]: for index, or a commit
> > >      or tree object name), and change the semantics in a future
> > >      major release for everybody, apologizing for potentially
> > >      breaking existing scripts; or
> >
> > That would be my first prio preference
>
> Mine too,  but I don't want to break anything.

It probably wont break anything which is not already broken.
It's somehow hard to imagine someone would use the
syntax to check for absence of a file in non-top-level
subdirectory. Besides, the existing syntax is ambiguous:
for anyone using pathname tab completion.

Besides, you can give a warning of ambiguity in case there are
equal relative and a top-level paths. Return the relative, but print
the warning, so people can fix their scripts.

> > > My preference is (2), (3) and then (1), but I do not have
> > > offhand a suggestion for a good metacharacter we could use.
> >
> > "./" :)
> Heh.  Yes, that works in the current patch.
>
> I'm really starting to like the idea of introducing "="
> as an alternative for ":" :
> "=" assumes relative paths, ":" assumes absolute (full) paths.
> That could be Junio's new metacharacter.

Just keep in mind: "once introduced, you'll never be able to remove it".
And I don't share your feelings regarding the special character, for instance.

I'd suggest to define a special character for _top_ level. Real pity
":/" is taken.

Re: [PATCH] Support ent:relative_path

[Dana How]

On 5/4/07, Alex Riesen <raa.lkml@gmail.com> wrote:
> > > > My preference is (2), (3) and then (1), but I do not have
> > > > offhand a suggestion for a good metacharacter we could use.
> > >
> > > "./" :)
> > Heh.  Yes, that works in the current patch.
> >
> > I'm really starting to like the idea of introducing "="
> > as an alternative for ":" :
> > "=" assumes relative paths, ":" assumes absolute (full) paths.
> > That could be Junio's new metacharacter.
>
> Just keep in mind: "once introduced, you'll never be able to remove it".
> And I don't share your feelings regarding the special character, for instance.
Indeed.

> I'd suggest to define a special character for _top_ level. Real pity
> ":/" is taken.
We could use ://fullpath for top level,
and :relpath for relative. Then "string" in :/string couldn't start with /,
which shouldn't be a problem (right?).  I've certainly seen double
slashes before;
perforce in fact uses them for the root of the repository (depot).

This all depends on deciding that :relpath should be the (incompatible)
new default, and I'm not sure that's going to happen.

-- 
Dana L. How  danahow@gmail.com  +1 650 804 5991 cell

Re: [PATCH] Support ent:relative_path

[Alex Riesen]

On 5/4/07, Dana How <danahow@gmail.com> wrote:
> > I'd suggest to define a special character for _top_ level. Real pity
> > ":/" is taken.
> We could use ://fullpath for top level,

No good. How'd you find a commit starting with "/" than? (without
changing ":/" syntax).

> and :relpath for relative. Then "string" in :/string couldn't start with /,
> which shouldn't be a problem (right?).  I've certainly seen double
> slashes before;
> perforce in fact uses them for the root of the repository (depot).

And I really hate perforce for its stupid redundancy (and changing of
meaning of well-known idioms: why should // be anything special
but plain top level or root?! Why the hell do they need them at if
you cannot use relative paths in client specs at all?! Why can't the
p4 command-line tool figure the fact from context or request the
context be provided by user?! IOW, Perforce is a real bad example
of how you do version control).

> This all depends on deciding that :relpath should be the (incompatible)
> new default, and I'm not sure that's going to happen.

If we are to stay that compatible, maybe ":./" for relative paths and the
old syntax left to mean top-level would the best choice for now.

Re: [PATCH] Support ent:relative_path

[Dana How]

On 5/4/07, Alex Riesen <raa.lkml@gmail.com> wrote:
> On 5/4/07, Dana How <danahow@gmail.com> wrote:
> > > I'd suggest to define a special character for _top_ level. Real pity
> > > ":/" is taken.
> > We could use ://fullpath for top level,
>
> No good. How'd you find a commit starting with "/" than? (without
> changing ":/" syntax).
Oh blechh.  get_sha1_oneline uses prefixcmp(), not strstr().
Are there are any commits in git or the kernel starting with "/" ?

> > and :relpath for relative. Then "string" in :/string couldn't start with /,
> > which shouldn't be a problem (right?).  I've certainly seen double
> > slashes before;
> > perforce in fact uses them for the root of the repository (depot).
>
> And I really hate perforce for its stupid redundancy (and changing of
> meaning of well-known idioms: why should // be anything special
> but plain top level or root?! Why the hell do they need them at if
> you cannot use relative paths in client specs at all?! Why can't the
> p4 command-line tool figure the fact from context or request the
> context be provided by user?! IOW, Perforce is a real bad example
> of how you do version control).
Whoa!  I'm stuck using perforce too; I'm not holding it up as a *big*
example.  I originally saw // meaning root in the Apollo DOMAIN
system,  so for that reason it makes sense to me.  I think it also
means network root in Windoze (well, \\ does ;-) )..

> > This all depends on deciding that :relpath should be the (incompatible)
> > new default, and I'm not sure that's going to happen.
>
> If we are to stay that compatible, maybe ":./" for relative paths and the
> old syntax left to mean top-level would the best choice for now.

Let's summarize so far:  I think everyone's convinced me we need
to be careful,  so this email will be more tedious than I'd like.

(a) :./relpath clearly inidicates relative path. [Also take :../relpath .]
(b) I'd like a more natural way to do :./relpath (e.g. :relpath),
     or at least a future path to such.
(c) We would like to avoid new special characters beyond ":".
     This means everything has to be done with "." and "/".
(d) We are left with the following patterns:
     1. :string
     2. :/string
     3. ://string

[ We need a clear way to say relative, a clear way to say absolute,
and the current :string can change from absolute to relative some time
in the future if we so decide. ]

Ideas for (d) 2&3:
I. Make :/string actually match the RE ^[/]*string,  and ://string a full path.
   The leading [/]* is a very small change to get_sha1_oneline().
   [Or change prefixcmp() to strstr() in get_sha1_oneline().]
   How often do commit messages start with / ?
II. Make :/string a full path, and ://string match ^string .
   Is changing the current :/string to ://string less painful/dangerous?
III. Make :/string match ^string when string has no slashes,
    :/string a full path when string does have slashes,
    and ://string match ^string . Hmm,  seems confusing.
Do you use :/string now?  Since it's a case-sensitive exact match,
I don't think I'd even use it.
I find idea (II) most natural: absolute paths have one /,
and string matches have 2 suggesting an RE.

Concerning the current :string , perhaps we could do the following.
There would be 2 internal fixed mode variables (NOT config variables) which
do the following.  The first controls whether this means an absolute
or relative path.  The second controls whether a warning message
is printed whenever the first is consulted to make a decision.  The
interpretation of :string is left as-is, but motivated janitors (like me
in this case) can use the second mode variable to change all
:string patterns to :/string or ://string in scripts,   letting us
switch over later
by changing one mode variable.

Someone mentioned DWIM for :string -- check both absolute and relative,
in that order for compatibility probably.  This seems a messy
definition to me.  Comments?

Anyway,  this is more involved than I'd hoped,
but it's good to think about consequences.

Thanks,
-- 
Dana L. How  danahow@gmail.com  +1 650 804 5991 cell

Re: [PATCH] Support ent:relative_path

[Alex Riesen]

On 5/4/07, Dana How <danahow@gmail.com> wrote:
> > > This all depends on deciding that :relpath should be the (incompatible)
> > > new default, and I'm not sure that's going to happen.
> >
> > If we are to stay that compatible, maybe ":./" for relative paths and the
> > old syntax left to mean top-level would the best choice for now.
>
> Let's summarize so far:  I think everyone's convinced me we need
> to be careful,  so this email will be more tedious than I'd like.
>
> (a) :./relpath clearly inidicates relative path. [Also take :../relpath .]

Ack

> (b) I'd like a more natural way to do :./relpath (e.g. :relpath),
>      or at least a future path to such.
> (c) We would like to avoid new special characters beyond ":".
>      This means everything has to be done with "." and "/".

And new semantics, if possible

> (d) We are left with the following patterns:
>      1. :string

Probably breaks something

>      2. :/string

Taken

>      3. ://string

Ugly (and breaks tab completion)

> [ We need a clear way to say relative, a clear way to say absolute,
> and the current :string can change from absolute to relative some time
> in the future if we so decide. ]
>
> Ideas for (d) 2&3:
> I. Make :/string actually match the RE ^[/]*string,  and ://string a full path.
>    The leading [/]* is a very small change to get_sha1_oneline().
>    [Or change prefixcmp() to strstr() in get_sha1_oneline().]
>    How often do commit messages start with / ?

How often should they start to justify any decision?

> II. Make :/string a full path, and ://string match ^string .
>    Is changing the current :/string to ://string less painful/dangerous?

Johannes?

> III. Make :/string match ^string when string has no slashes,
>     :/string a full path when string does have slashes,
>     and ://string match ^string . Hmm,  seems confusing.

Very/

> Do you use :/string now?  Since it's a case-sensitive exact match,

No. It just exists, AFAICS

> I don't think I'd even use it.
> I find idea (II) most natural: absolute paths have one /,
> and string matches have 2 suggesting an RE.

I think, I'd be convinced by this one too, if not for "//" making
no sense anywhere but root accidentally typed twice.
I'd even sacrifice (or change) the search syntax for good.

Re: [PATCH] Support ent:relative_path

[Johannes Schindelin]

Hi,

On Fri, 4 May 2007, Alex Riesen wrote:

> On 5/4/07, Dana How <danahow@gmail.com> wrote:
>
> > II. Make :/string a full path, and ://string match ^string .
> >    Is changing the current :/string to ://string less painful/dangerous?
> 
> Johannes?

Huh? Hello!

I did not follow this thread closely...

Having said that, I think ":./bla/blub" is the most intuitive, and the 
least breaking.

You _could_ change the current :/ notation, but _why_?

Ciao,
Dscho

Re: [PATCH] Support ent:relative_path

[Alex Riesen]

Johannes Schindelin, Fri, May 04, 2007 21:00:53 +0200:
> Having said that, I think ":./bla/blub" is the most intuitive, and the 
> least breaking.
> 
> You _could_ change the current :/ notation, but _why_?

To use the syntax for full path starting from root of repo.
It looks "most intuitive" for this.

Re: [PATCH] Support ent:relative_path

[Johannes Sixt]

Alex Riesen wrote:
> On 5/4/07, Junio C Hamano <junkio@cox.net> wrote:
> > My preference is (2), (3) and then (1), but I do not have
> > offhand a suggestion for a good metacharacter we could use.
> 
> "./" :)

+1, without the :)
and ../ should DWIM, too.

-- Hannes

Re: [PATCH] Support ent:relative_path

[Junio C Hamano]

Johannes Sixt <J.Sixt@eudaptics.com> writes:

> Alex Riesen wrote:
>> On 5/4/07, Junio C Hamano <junkio@cox.net> wrote:
>> > My preference is (2), (3) and then (1), but I do not have
>> > offhand a suggestion for a good metacharacter we could use.
>> 
>> "./" :)
>
> +1, without the :)
> and ../ should DWIM, too.

I think these feel more or less natural (except that we do not
say ":/fullpath" and instead say ":fullpath" which is
unfortunate).

In the hindsight, if we had the perfect vision into the future,
we would have made <path> relative to where you are when we
initially did "<something>:<path>", with obvious semantics for
things like "<something>:../<path>" and "<something>:/<path>".

We didn't.  Further, we made a mistake to make :/ to mean
something completely unrelated.  My bad.

So I think "./" is the best compromise in the meantime.

With your suggestion, we can train people's fingers to type "./"
now, and perhaps later in one of those big feature release like
the 1.5.0 was, we could switch to "default to relative".

At the same time of that big "UI correction", we could make
"<something>:/<path>" to mean "full path in commit (or
index/stage) no matter where I am".

The current ":/<string>" is about going back, looking for the
string, so it should not have used '/'; instead it should have
been "<something>:?<string>".  Maybe we could fix it by start
accepting ":?" now (in addition to ":/"), give a big fat warning
about ":/" going to mean a different thing, and encouraging
users to use the question-mark form, in preparation for the big
"UI correction".

Do people like that plan?

(soon after 1.5.2)

 - start accepting "<something>:./<path>" as "relative to where I am".

 - start accepting "<something>:?<string>" as "look back to find
   the string".

 - clearly explain the plan and prepare the users.

(a big release in the future, perhaps 1.6.0)

 - "<something>:<path>" becomes relative to where you are.
 - stop "<something>:/<string>" and start "<something>:/<fullpath>".

Re: [PATCH] Support ent:relative_path

[Dana How]

On 5/4/07, Junio C Hamano <junkio@cox.net> wrote:
> Johannes Sixt <J.Sixt@eudaptics.com> writes:
>
> > Alex Riesen wrote:
> >> On 5/4/07, Junio C Hamano <junkio@cox.net> wrote:
> >> > My preference is (2), (3) and then (1), but I do not have
> >> > offhand a suggestion for a good metacharacter we could use.
> >>
> >> "./" :)
> >
> > +1, without the :)
> > and ../ should DWIM, too.
>
> I think these feel more or less natural (except that we do not
> say ":/fullpath" and instead say ":fullpath" which is
> unfortunate).
>
> In the hindsight, if we had the perfect vision into the future,
> we would have made <path> relative to where you are when we
> initially did "<something>:<path>", with obvious semantics for
> things like "<something>:../<path>" and "<something>:/<path>".
>
> We didn't.  Further, we made a mistake to make :/ to mean
> something completely unrelated.  My bad.
>
> So I think "./" is the best compromise in the meantime.
>
> With your suggestion, we can train people's fingers to type "./"
> now, and perhaps later in one of those big feature release like
> the 1.5.0 was, we could switch to "default to relative".
>
> At the same time of that big "UI correction", we could make
> "<something>:/<path>" to mean "full path in commit (or
> index/stage) no matter where I am".
>
> The current ":/<string>" is about going back, looking for the
> string, so it should not have used '/'; instead it should have
> been "<something>:?<string>".  Maybe we could fix it by start
> accepting ":?" now (in addition to ":/"), give a big fat warning
> about ":/" going to mean a different thing, and encouraging
> users to use the question-mark form, in preparation for the big
> "UI correction".
>
> Do people like that plan?
>
> (soon after 1.5.2)
>
>  - start accepting "<something>:./<path>" as "relative to where I am".
>
>  - start accepting "<something>:?<string>" as "look back to find
>    the string".
>
>  - clearly explain the plan and prepare the users.
>
> (a big release in the future, perhaps 1.6.0)
>
>  - "<something>:<path>" becomes relative to where you are.
>  - stop "<something>:/<string>" and start "<something>:/<fullpath>".

I will resubmit the patch following Junio's plan.
It will include the "1.6.0" features but disabled.

I would just request that we expedite,  if possible,
at least the later switch from "<something>:/<string>"
to "<something>:/<fullpath>",
since with the patch _and_ the latter behavior enabled,
scripts can be updated to always insert "./" or "/" and
their intent will always be clear.

Thanks,
-- 
Dana L. How  danahow@gmail.com  +1 650 804 5991 cell

Re: [PATCH] Support ent:relative_path

[Johannes Schindelin]

Hi,

On Fri, 4 May 2007, Junio C Hamano wrote:

> (a big release in the future, perhaps 1.6.0)
> 
>  - "<something>:<path>" becomes relative to where you are.

FWIW I still find that unintuitive. I know "<something>:<path>" from ssh, 
and there it does not change meaning depending on where I am. IMHO in most 
cases you want to use git-diff anyway, which _does_ honour the current 
relative path.

Ciao,
Dscho

Re: [PATCH] Support ent:relative_path

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> FWIW I still find that unintuitive. I know "<something>:<path>" from ssh, 
> and there it does not change meaning depending on where I am. IMHO in most 
> cases you want to use git-diff anyway, which _does_ honour the current 
> relative path.

There, its meaning is relative to where you are, namely "$HOME".

Re: [PATCH] Support ent:relative_path

[Johannes Schindelin]

Hi,

On Fri, 4 May 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > FWIW I still find that unintuitive. I know "<something>:<path>" from ssh, 
> > and there it does not change meaning depending on where I am. IMHO in most 
> > cases you want to use git-diff anyway, which _does_ honour the current 
> > relative path.
> 
> There, its meaning is relative to where you are, namely "$HOME".

No, it is relative to where I am _at the other end_. If I "cd /tmp", it 
still is relative to $HOME.

Now, what you want to do is changing the meaning of v1.5.1:Makefile, 
depending if you "cd Documentation/"ed or not.

For me, "v1.5.1:" means something similar to ssh: it is a distant 
revision. It is not a complete filesystem. I think of revisions as 
something more general than a directory, but less general than a 
filesystem. And thus, it makes perfect sense to me that "v1.5.1:Makefile" 
means the main Makefile, no matter where I am in the current repository.

Now, I agree that often you want to compare some file in the current 
directory to the corresponding file in a certain revision. That is why 
git-diff has a different idea, and indeed, a different notation, too.

Ciao,
Dscho

Re: [PATCH] Support ent:relative_path

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> For me, "v1.5.1:" means something similar to ssh: it is a distant 
> revision. It is not a complete filesystem. I think of revisions as 
> something more general than a directory, but less general than a 
> filesystem. And thus, it makes perfect sense to me that "v1.5.1:Makefile" 
> means the main Makefile, no matter where I am in the current repository.

I see merits in both sides' arguments.  

Saying "path out of THIS version" anchors your mindset at the
top of that version, so in that sense v1.5.1:Makefile should
mean the toplevel no matter where you are.  However...

> Now, I agree that often you want to compare some file in the current 
> directory to the corresponding file in a certain revision. That is why 
> git-diff has a different idea, and indeed, a different notation, too.

... don't you expect "git show HEAD@{yesterday}:git.txt" while
in Documentation/ subdirectory to work?  I wonder if the current
alternative "git show HEAD@{yesterday}:Documentation/git.txt" a
bit unintuitive.

Re: [PATCH] Support ent:relative_path

[Johannes Schindelin]

Hi,

On Fri, 4 May 2007, Junio C Hamano wrote:

> ... don't you expect "git show HEAD@{yesterday}:git.txt" while
> in Documentation/ subdirectory to work?

Actually, no. I expect "git diff HEAD@{yesterday} git.txt" to work fine, 
and "git show HEAD@{yesterday}:git.c" also, both when in Documentation/, 
because I expect "git show HEAD@{yesterday}:git.c" to work _also_ when I 
set GIT_DIR=/path/to/my/bare-repo.git/ before that, while I do not expect 
the call to git-diff to work (for obvious reasons).

Ciao,
Dscho

Re: [PATCH] Support ent:relative_path

[Alex Riesen]

Junio C Hamano, Fri, May 04, 2007 19:17:39 +0200:
> Do people like that plan?
> 
> (soon after 1.5.2)
> 
>  - start accepting "<something>:./<path>" as "relative to where I am".
> 
>  - start accepting "<something>:?<string>" as "look back to find
>    the string".

Still don't like the feature. It makes things special: suddenly we
have to be careful about not having files with a "?" as their first
symbol (well, scripts always can prepend "./" or "/")

And it is hard to use: only prefix, must be unambiguous and is
case sensitive.

>  - clearly explain the plan and prepare the users.
> 
> (a big release in the future, perhaps 1.6.0)
> 
>  - "<something>:<path>" becomes relative to where you are.
>  - stop "<something>:/<string>" and start "<something>:/<fullpath>".

I like the plan.

Re: [PATCH] Support ent:relative_path

[Jakub Narebski]

Junio C Hamano wrote:

> (soon after 1.5.2)
> 
>  - start accepting "<something>:./<path>" as "relative to where I am".
>  - start accepting "<something>:?<string>" as "look back to find
>    the string".
>  - clearly explain the plan and prepare the users.
> 
> (a big release in the future, perhaps 1.6.0)
> 
>  - "<something>:<path>" becomes relative to where you are.
>  - stop "<something>:/<string>" and start "<something>:/<fullpath>".

I'm not sure about "<tree-ish>:<path>" with <path> being relative by
default. For me it is <path> in <tree-ish> (like in 
"git-ls-tree -r <tree-ish>" result).

The "<tree-ish>:./<path>" is a good syntax I think.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH] Support ent:relative_path

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> I'm not sure about "<tree-ish>:<path>" with <path> being relative by
> default. For me it is <path> in <tree-ish> (like in 
> "git-ls-tree -r <tree-ish>" result).

That's right (and Dscho is also).

"v1.5.1:git.c" IS "git.c that appears at the toplevel of
v1.5.1's tree."

Ok, for now let's forget about this relative stuff.

Re: [PATCH] Support ent:relative_path

[Dana How]

On 5/4/07, Junio C Hamano <junkio@cox.net> wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> > I'm not sure about "<tree-ish>:<path>" with <path> being relative by
> > default. For me it is <path> in <tree-ish> (like in
> > "git-ls-tree -r <tree-ish>" result).
>
> That's right (and Dscho is also).
>
> "v1.5.1:git.c" IS "git.c that appears at the toplevel of
> v1.5.1's tree."
>
> Ok, for now let's forget about this relative stuff.

Hmm, most of the work I do in the parts of our
perforce repository I want to convert to git is
far enough down that the paths have 6 in-repo path components.
I don't want to type all those when I want to fetch an
older version with git-show.  Everything I do is relative.
In fact,  I think perforce supports typing absolute paths,
(using an 8-character prefix!) but I have never used it,
nor would I if it were shorter.
-- 
Dana L. How  danahow@gmail.com  +1 650 804 5991 cell

Re: [PATCH] Support ent:relative_path

[Jakub Narebski]

Dana How wrote:
> On 5/4/07, Junio C Hamano <junkio@cox.net> wrote:
>> Jakub Narebski <jnareb@gmail.com> writes:

>>> I'm not sure about "<tree-ish>:<path>" with <path> being relative by
>>> default. For me it is <path> in <tree-ish> (like in
>>> "git-ls-tree -r <tree-ish>" result).
>>
>> That's right (and Dscho is also).
>>
>> "v1.5.1:git.c" IS "git.c that appears at the toplevel of
>> v1.5.1's tree."
>>
>> Ok, for now let's forget about this relative stuff.
> 
> Hmm, most of the work I do in the parts of our
> perforce repository I want to convert to git is
> far enough down that the paths have 6 in-repo path components.
> I don't want to type all those when I want to fetch an
> older version with git-show.  Everything I do is relative.
> In fact,  I think perforce supports typing absolute paths,
> (using an 8-character prefix!) but I have never used it,
> nor would I if it were shorter.

I think the consensus is to use <tree-ish>:./<relative-path> for 
relative paths, and <tree-ish>:<path> for absolute (well, in the 
meaning that it counts from top of _tree-ish_, and tree-ish needs
not to be top tree / commit tree).

And I think 6 in-repo path components means not very well thought 
directory hierarchy, I think...

-- 
Jakub Narebski
Poland

Re: [PATCH] Support ent:relative_path

[Junio C Hamano]

"Dana How" <danahow@gmail.com> writes:

> On 5/4/07, Junio C Hamano <junkio@cox.net> wrote:
>> Jakub Narebski <jnareb@gmail.com> writes:
>> > I'm not sure about "<tree-ish>:<path>" with <path> being relative by
>> > default. For me it is <path> in <tree-ish> (like in
>> > "git-ls-tree -r <tree-ish>" result).
>>
>> That's right (and Dscho is also).
>>
>> "v1.5.1:git.c" IS "git.c that appears at the toplevel of
>> v1.5.1's tree."
>>
>> Ok, for now let's forget about this relative stuff.
>
> Hmm, most of the work I do in the parts of our
> perforce repository I want to convert to git is
> far enough down...

The key word in my statement is "for now".  I simply declared
that I do not want to think about it further until 1.5.2 final,
that's all.

I am not opposed to give an escape hatch for your problem; I am
quite in favor of that, and I think Alex/J6t's "./" syntax is a
reasonable compromise.

Re: [PATCH] Support ent:relative_path

[Alex Riesen]

On 5/4/07, Dana How <danahow@gmail.com> wrote:
> +       if (!strcmp(var, "core.relativepaths")) {
> +               assume_relative_paths = git_config_bool(var, value);
> +               return 0;
> +       }

Maybe we should use relative path always, unconditionally.