[PATCH] _git: offer changed files relative to current directory

[PATCH] _git: offer changed files relative to current directory

[Alexey I. Froloff]

__git_changed_files() behavior should match git-commit, pass --relative
to git-diff-index then looking for changed files.

Signed-off-by: Alexey I. Froloff <raorn@altlinux.org>
---
 Completion/Unix/Command/_git |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index e483133..dd3617b 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -2826,7 +2826,7 @@ __git_unmerged_files () {
 __git_changed_files () {
   local -a files
 
-  files=(${(ps:\0:)"$(_call_program files git diff-index -z --name-only --no-color HEAD 2>/dev/null)"})
+  files=(${(ps:\0:)"$(_call_program files git diff-index -z --name-only --no-color --relative HEAD 2>/dev/null)"})
   __git_command_successful || return
 
   _wanted files expl 'index file' _multi_parts $@ - / files
-- 
1.6.5.3

Re: [PATCH] _git: offer changed files relative to current directory

[Štěpán Němec]

On Sat, Dec 05, 2009 at 03:32:43AM +0300, Alexey I. Froloff wrote:
> __git_changed_files() behavior should match git-commit, pass --relative
> to git-diff-index then looking for changed files.
> 
> Signed-off-by: Alexey I. Froloff <raorn@altlinux.org>
> ---
>  Completion/Unix/Command/_git |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
> index e483133..dd3617b 100644
> --- a/Completion/Unix/Command/_git
> +++ b/Completion/Unix/Command/_git
> @@ -2826,7 +2826,7 @@ __git_unmerged_files () {
>  __git_changed_files () {
>    local -a files
>  
> -  files=(${(ps:\0:)"$(_call_program files git diff-index -z --name-only --no-color HEAD 2>/dev/null)"})
> +  files=(${(ps:\0:)"$(_call_program files git diff-index -z --name-only --no-color --relative HEAD 2>/dev/null)"})
>    __git_command_successful || return
>  
>    _wanted files expl 'index file' _multi_parts $@ - / files
> -- 
> 1.6.5.3
> 
No, this is wrong as far as I recall.
I was solving the same problem before some time. Your solution (i.e.
git-diff-index --relative) only displays the changed files under a
subdirectory, which is incorrect when not in the repository root.

I solved the problem by writing a 50 line or so "ueberfunction" which
manually relativizes the paths returned by git-diff-index to the correct
form required by git-commit (i.e. relative to the current directory, no
matter what it is -- and still displaying all the changed files not only
under the current directory, but from the whole repository).  Seems to
work, but I don't really feel very "proud" of it so I haven't tried to
share it so far.

    Štěpán

Re: [PATCH] _git: offer changed files relative to current directory

[Alexey I. Froloff]

[-- Attachment #1.1: Type: text/plain, Size: 1329 bytes --]

On Sat, Dec 05, 2009 at 01:07:54PM +0100, ??t??p??n N??mec wrote:
> No, this is wrong as far as I recall.
At least this is better than current behavior.

> I was solving the same problem before some time. Your solution (i.e.
> git-diff-index --relative) only displays the changed files under a
> subdirectory, which is incorrect when not in the repository root.
Same problem exists for __git_*_files (__git_files).

> I solved the problem by writing a 50 line or so "ueberfunction" which
> manually relativizes the paths returned by git-diff-index to the correct
> form required by git-commit
You can get current subdirectory with "git rev-parse
--show-prefix", then all "absolute" paths must be turned into
relative.  On my system there is "relative" program (comes with
rpm-build package), so the code would look like:

local -a rawfiles files
local f prefix

prefix=$(_call_program gitprefix git rev-parse --show-prefix 2>/dev/null)
__git_command_successful || return

rawfiles=( ... git ls-files or git diff-index call ... )

for f in $rawfiles; do
  files+=( $(relative /$prefix /$f) )
done

_wanted ...

Just need pure-zsh "relative" implementation.  relative.c
attached.

P.S. I don't like this for loop.  Can this be done with single
expansion expression?

-- 
Regards,
Sir Raorn.

[-- Attachment #1.2: relative.c --]
[-- Type: text/plain, Size: 3292 bytes --]

/*
  Copyright (C) 2001  Dmitry V. Levin <ldv@altlinux.org>

  This program is free software; you can redistribute it and/or modify
  it under the terms of the GNU General Public License as published by
  the Free Software Foundation; either version 2 of the License, or
  (at your option) any later version.

  This program is distributed in the hope that it will be useful,
  but WITHOUT ANY WARRANTY; without even the implied warranty of
  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
  GNU General Public License for more details.

  You should have received a copy of the GNU General Public License
  along with this program; if not, write to the Free Software
  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <errno.h>
#include <error.h>
#include <sys/param.h>

static void *
xmalloc(size_t size)
{
	void   *r = malloc(size);

	if (!r)
		error(EXIT_FAILURE, errno, "malloc: allocating %lu bytes",
		      (unsigned long) size);
	return r;
}

static char *
xstrdup(const char *s)
{
	size_t  len = strlen(s);
	char   *r = xmalloc(len + 1);

	memcpy(r, s, len + 1);
	return r;
}

static void __attribute__ ((noreturn))
result(const char *str)
{
	puts(str);
	exit(0);
}

static const char *
normalize(char *str)
{
	char   *p;
	size_t  len;

	for (p = strstr(str, "//"); p; p = strstr(str, "//"))
		memmove(p, p + 1, strlen(p));

	for (p = strstr(str, "/./"); p; p = strstr(str, "/./"))
		memmove(p, p + 2, strlen(p + 1));

	len = strlen(str);
	if ((len >= 2) && ('/' == str[len - 2]) && ('.' == str[len - 1]))
		str[len - 1] = '\0';

	return str;
}

static void
strip_trailing(char *str, const char sym)
{
	char   *p;

	for (p = strrchr(str, sym); p && (p >= str) && (sym == *p); --p)
		*p = '\0';
}

static const char *
base_name(const char *name)
{
	const char *p = strrchr(name, '/');

	if (p)
		return p + 1;
	else
		return name;
}

static char *
lookup_back(const char *str, const char sym, const char *pos)
{
	for (; pos >= str; --pos)
		if (sym == *pos)
			return (char *) pos;

	return 0;
}

int
main(int ac, char *av[])
{
	const char *orig_what;
	char   *what_p, *to_p, *res;
	char   *what, *to;

	if (ac < 3)
	{
		fprintf(stderr, "Usage: %s <what> <to>\n",
			program_invocation_short_name);
		return 1;
	}

	what = xstrdup(av[1]);
	to = xstrdup(av[2]);

	orig_what = normalize(av[1]);
	normalize(av[2]);

	what = xstrdup(av[1]);
	to = xstrdup(av[2]);

	if ('/' != *what)
		result(what);

	if ('/' != *to)
		error(EXIT_FAILURE, 0,
		      "destination must be absolute filename");

	strip_trailing(what, '/');
	strip_trailing(to, '/');

	for (what_p = what, to_p = to; *what_p && *to_p; ++what_p, ++to_p)
		if (*what_p != *to_p)
			break;

	if (!*what_p && !*to_p)
		result(base_name(orig_what));

	res = xmalloc(strlen(what) + strlen(to) * 3 / 2 + 3);

	if (('/' == *what_p) && !*to_p)
		result(orig_what + (++what_p - what));

	if ('/' != *to_p || *what_p)
	{
		what_p = lookup_back(what, '/', what_p - 1);
		strcpy(res, "..");
	} else
	{
		res[0] = '\0';
	}

	for (; *to_p; ++to_p)
	{
		if ('/' == *to_p)
		{
			if (*res)
				strcat(res, "/..");
			else
				strcpy(res, "..");
		}
	}

	strcat(res, orig_what + (what_p - what));
	result(res);
}

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] _git: offer changed files relative to current directory

[Alexey I. Froloff]

[-- Attachment #1: Type: text/plain, Size: 672 bytes --]

On Sat, Dec 05, 2009 at 03:42:58PM +0300, Alexey I. Froloff wrote:
> Just need pure-zsh "relative" implementation.
Here's my quick'n'dirty code:

relate()
{
    local -a what to res

    what=( ${(ps:/:)"${${${${1//\/\///}//\/.\///}%%/.}%%/}"} )
    to=( ${(ps:/:)"${${${${2//\/\///}//\/.\///}%%/.}%%/}"} )

    while (( $#what > 0 )) && (( $#to > 0 )) && [[ $what[1] == $to[1] ]]; do
	what[1]=()
	to[1]=()
    done

    while (( $#to > 0 )); do
	res+=..
	to[1]=()
    done

    res=( "$res[@]" "$what[@]" )

    echo ${(pj:/:)res}
}

relate Completion/Unix/Command/_git Functions/ 
../Completion/Unix/Command/_git

-- 
Regards,
Sir Raorn.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] _git: offer changed files relative to current directory

[Štěpán Němec]

On Sat, Dec 05, 2009 at 06:12:03PM +0300, Alexey I. Froloff wrote:
> On Sat, Dec 05, 2009 at 03:42:58PM +0300, Alexey I. Froloff wrote:
> > Just need pure-zsh "relative" implementation.
> Here's my quick'n'dirty code:
> 
I'm not sure about the exact purpose of this function, but it doesn't
seem correct to me:
> relate()
> {
>     local -a what to res
> 
>     what=( ${(ps:/:)"${${${${1//\/\///}//\/.\///}%%/.}%%/}"} )
>     to=( ${(ps:/:)"${${${${2//\/\///}//\/.\///}%%/.}%%/}"} )
> 
>     while (( $#what > 0 )) && (( $#to > 0 )) && [[ $what[1] == $to[1] ]]; do
> 	what[1]=()
> 	to[1]=()
>     done
It is not unlikely to have more same-named directories in different
parts of directory tree. IIUC, the code above won't always work
correctly in such situation (i.e. it will remove the directory from the
path, wrongly assuming it is the same directory for both paths).

I believe the only correct way to solve the path relativization problem
for arbitrary input is to use the *absolute* paths.

In any case, it would be more useful/comprehensible (to me at least) if
you included the solution with the former patch to solve the actual
problem.


Anyway, here is what I've been succesfully using for some time; don't
laugh your teeth off:

------- 8< -------
From: Štěpán Němec <stepnem@gmail.com>
Date: Sat, 5 Dec 2009 17:42:56 +0100
Subject: [PATCH] _git: Fix commit path completion.

Paths returned by git-diff-index are relative to repository root; make
them relative to the current directory (i.e. how git-commit requires
them), using a `munge_paths' monsterfunction.

Signed-off-by: Štěpán Němec <stepnem@gmail.com>
---
 _git |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 54 insertions(+), 1 deletions(-)

diff --git a/_git b/_git
index e483133..b426612 100644
--- a/_git
+++ b/_git
@@ -2824,12 +2824,65 @@ __git_unmerged_files () {
 #this is for git-commit which can take files both git-added and not
 (( $+functions[__git_changed_files] )) ||
 __git_changed_files () {
+  # this function is needed to transform paths relative to the repo base dir
+  # (as returned by diff-index) into the form relative to the current directory
+  # :|
+  munge_paths () {
+    # SN FIXME is this needed? How come I don't see emulate -R anywhere in this file?
+    emulate -LR zsh
+    local cwd scnt scnta scntc
+    local -a abs paths
+    cwd=${"$(pwd)":a}
+    base=${${"$(git rev-parse --git-dir)":a}%.git}
+    abs=($base${^files})
+    scntc=${#${cwd//[^\/]}}
+    for ((i=1; i <= $#abs; i++)); do
+      scnta=${#${abs[$i]//[^\/]}}
+      scnt=$(($scntc - $scnta))
+      if ((scnt >= 0)); then
+      paths[$i]="$abs[$i]:t"
+        for ((j=0; j <= $scnt; j++)); do
+          paths[$i]="../$paths[$i]"
+        done
+      else
+        if [[ $abs[$i] == $cwd* ]]; then
+          paths[$i]="${${abs[$i]}#$cwd/}"
+        else
+          # longest common prefix
+          lcp () {
+            local lcp
+            for ((i=1; i<=$#1; i++)); do
+              if [[ ${1[i]} == ${2[i]} ]]; then
+                lcp+=${1[i]}
+              else
+                break
+              fi
+            done
+            echo "$lcp"
+          }
+          local common="$(lcp $cwd $abs[$i])"
+          local prefix=""
+          paths[$i]="${${abs[$i]}#$common}"
+          local cwdr="${${cwd}#$common}"
+          scntcr=${#${cwdr//[^\/]}}
+          for ((j=0; j <= $scntcr; j++)); do
+            prefix="../$prefix"
+          done
+          paths[$i]="$prefix$paths[$i]"
+        fi
+      fi
+    done
+    print ${(pj:\0:)paths}
+  }
+
   local -a files

   files=(${(ps:\0:)"$(_call_program files git diff-index -z --name-only --no-color HEAD 2>/dev/null)"})
   __git_command_successful || return
+  files=(${(ps:\0:)"$(munge_paths)"})

-  _wanted files expl 'index file' _multi_parts $@ - / files
+  _description files expl 'Changed files'
+  compadd "$expl[@]" - "$files[@]"
 }

 (( $+functions[__git_tree_files] )) ||
--

Re: [PATCH] _git: offer changed files relative to current directory

[Alexey I. Froloff]

[-- Attachment #1: Type: text/plain, Size: 2044 bytes --]

On Sat, Dec 05, 2009 at 06:00:53PM +0100, ??t??p??n N??mec wrote:
> I'm not sure about the exact purpose of this function, but it doesn't
> seem correct to me:
Yes, my code requires some explanations.

$what is the file we got from "git tree-index" or "git ls-files
--full-name -- $(git rev-parse --show-cdup)", we can assume it's
a full path from the root of repository.

$to is the path we got from "git rev-parse --show-prefix".

    what=( ${(s:/:)"${${${${1//\/\///}//\/.\///}%%/.}%%/}"} )
    to=( ${(s:/:)"${${${${2//\/\///}//\/.\///}%%/.}%%/}"} )

Some paranoid code that strips "/./", converts multiple slashes
to one and strips "/." and "/" from right.  Then path is split to
components for easier processing.

BTW, if prefix is empty string, $to will be equal to ( "" ) which
will add unneeded "../" later (my bug).

    while (( $#what > 0 )) && (( $#to > 0 )) && [[ $what[1] == $to[1] ]]; do
	what[1]=()
	to[1]=()
    done

This loop strips common prefix from $what and $to, we wont need
it in relative path.

    while (( $#to > 0 )); do
	res+=..
	to[1]=()
    done

For each path component left in $to ascend one level up.

    res=( "$res[@]" "$what[@]" )

Add path components left in $what.

> It is not unlikely to have more same-named directories in different
> parts of directory tree. IIUC, the code above won't always work
> correctly in such situation (i.e. it will remove the directory from the
> path, wrongly assuming it is the same directory for both paths).
No, first loop will only remove prefix common for both paths.

> I believe the only correct way to solve the path relativization problem
> for arbitrary input is to use the *absolute* paths.
These paths _are_ absolute since it is impossible to ascend
outside of git repository.  Hope we can assume so ;-)

> In any case, it would be more useful/comprehensible (to me at least) if
> you included the solution with the former patch to solve the actual
> problem.
A bit later.

-- 
Regards,
Sir Raorn.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[PATCH] _git: offer files relative to current directory

[Alexey I. Froloff]

When offering repository files (cached, deleted, changed, etc), make
sure that paths are relative to current directory, as git expects.

Signed-off-by: Alexey I. Froloff <raorn@altlinux.org>
---
 Completion/Unix/Command/_git |   61 +++++++++++++++++++++++++++++++++++++++---
 1 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index e483133..5371fd5 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -2773,19 +2773,70 @@ __git_stages () {
   __git_guard $* "[[:digit:]]#" 'stage'
 }
 
+(( $+functions[__git_files_relative] )) ||
+__git_files_relative () {
+  local rawfiles files file f_parts prefix p_parts tmp
+
+  prefix=$(_call_program gitprefix git rev-parse --show-prefix 2>/dev/null)
+  __git_command_successful || return
+
+  # Empty prefix, no modifications
+  if (( $#prefix == 0 )); then
+    print $1
+    return
+  fi
+
+  rawfiles=(${(ps:\0:)1})
+  files=()
+
+  # Now we assume that we've given "absolute" paths list with "root"
+  # being repository top directory.  $prefix is also "absolute" path.
+  for file in $rawfiles; do
+    # Collapse "/./" and "//", strip "/." and "/" from tail (I know,
+    # this is a bit paranoid).
+    f_parts=(${(s:/:)"${${${${file//\/\///}//\/.\///}%%/.}%%/}"})
+    p_parts=(${(s:/:)"${${${${prefix//\/\///}//\/.\///}%%/.}%%/}"})
+    tmp=()
+
+    # Strip common path prefix.
+    while (( $#f_parts > 0 )) && (( $#p_parts > 0 )) && [[ $f_parts[1] == $p_parts[1] ]]; do
+      f_parts[1]=()
+      p_parts[1]=()
+    done
+
+    # If prefix still not empty, ascend up.
+    while (( $#p_parts > 0 )); do
+	tmp+=..
+	p_parts[1]=()
+    done
+
+    # Add remaining path.
+    tmp=("$tmp[@]" "$f_parts[@]")
+
+    files+=${(j:/:)tmp}
+  done
+
+  print ${(pj:\0:)files}
+}
+
 (( $+functions[__git_files] )) ||
 __git_files () {
-  local expl files ls_opts opts gitdir
+  local expl files ls_opts opts gitdir gitcdup
 
   zparseopts -D -E -a opts -- -cached -deleted -modified -others -ignored -unmerged -killed
 
   gitdir=$(_call_program gitdir git rev-parse --git-dir 2>/dev/null)
   __git_command_successful || return
 
+  gitcdup=$(_call_program gitcdup git rev-parse --show-cdup 2>/dev/null)
+  __git_command_successful || return
+
   ls_opts=("--exclude-per-directory=.gitignore")
   [[ -f "$gitdir/info/exclude" ]] && ls_opts+="--exclude-from=$gitdir/info/exclude"
 
-  files=(${(ps:\0:)"$(_call_program files git ls-files -z $ls_opts $opts 2>/dev/null)"})
+  files=$(_call_program files git ls-files -z --full-name $ls_opts $opts -- $gitcdup 2>/dev/null)
+  __git_command_successful || return
+  files=(${(ps:\0:)"$(__git_files_relative $files)"})
   __git_command_successful || return
 
   _wanted files expl 'index file' _multi_parts $@ - / files
@@ -2824,9 +2875,11 @@ __git_unmerged_files () {
 #this is for git-commit which can take files both git-added and not
 (( $+functions[__git_changed_files] )) ||
 __git_changed_files () {
-  local -a files
+  local files
 
-  files=(${(ps:\0:)"$(_call_program files git diff-index -z --name-only --no-color HEAD 2>/dev/null)"})
+  files=$(_call_program files git diff-index -z --name-only --no-color HEAD 2>/dev/null)
+  __git_command_successful || return
+  files=(${(ps:\0:)"$(__git_files_relative $files)"})
   __git_command_successful || return
 
   _wanted files expl 'index file' _multi_parts $@ - / files
-- 
1.6.5.3

Re: [PATCH] _git: offer files relative to current directory

[Alexey I. Froloff]

[-- Attachment #1: Type: text/plain, Size: 254 bytes --]

On Mon, Dec 07, 2009 at 12:52:33AM +0300, Alexey I. Froloff wrote:
> When offering repository files (cached, deleted, changed, etc), make
> sure that paths are relative to current directory, as git expects.
Any comments?

-- 
Regards,
Sir Raorn.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] _git: offer files relative to current directory

[Štěpán Němec]

On Tue, Dec 08, 2009 at 01:40:57AM +0300, Alexey I. Froloff wrote:
> On Mon, Dec 07, 2009 at 12:52:33AM +0300, Alexey I. Froloff wrote:
> > When offering repository files (cached, deleted, changed, etc), make
> > sure that paths are relative to current directory, as git expects.
> Any comments?
> 
> -- 
> Regards,
> Sir Raorn.

Sorry, I have some other work currently.

Just now I tested your patch, and it doesn't work for me:

In a repository where `git status' reports the following changed files:
#       modified:   doc/tags
#       modified:   filetype.vim
#       modified:   ftplugin/vim.vim
#       modified:   ftplugin/zsh.vim
#       modified:   indent/sh.vim
#       modified:   plugin/utl.vim
#       modified:   snippets/_.snippets
#       modified:   syntax/elisp.vim
#       modified:   syntax/sh.vim
#       modified:   syntax/vim.vim

`git commit <tab>' only suggests:
doc           filetype.vim  ftplugin      indent        plugin        snippets      syntax

which is obviously wrong; so my current solution is still functionally
better :-/

Regards,

    Štěpán

Re: [PATCH] _git: offer files relative to current directory

[Nikolai Weibull]

On Tue, Dec 8, 2009 at 11:48, Štěpán Němec <stepnem@gmail.com> wrote:

> In a repository where `git status' reports the following changed files:
> #       modified:   doc/tags
> #       modified:   filetype.vim
> #       modified:   ftplugin/vim.vim
> #       modified:   ftplugin/zsh.vim
> #       modified:   indent/sh.vim
> #       modified:   plugin/utl.vim
> #       modified:   snippets/_.snippets
> #       modified:   syntax/elisp.vim
> #       modified:   syntax/sh.vim
> #       modified:   syntax/vim.vim
>
> `git commit <tab>' only suggests:
> doc           filetype.vim  ftplugin      indent        plugin        snippets      syntax

Why’s that wrong?  Completing git commit syntax hopefully adds a slash
and lets you complete the files in the sub-folder, right?

Re: [PATCH] _git: offer files relative to current directory

[Štěpán Němec]

On Tue, Dec 08, 2009 at 11:50:18AM +0100, Nikolai Weibull wrote:
> On Tue, Dec 8, 2009 at 11:48, Štěpán Němec <stepnem@gmail.com> wrote:
> 
> > In a repository where `git status' reports the following changed files:
> > #       modified:   doc/tags
> > #       modified:   filetype.vim
> > #       modified:   ftplugin/vim.vim
> > #       modified:   ftplugin/zsh.vim
> > #       modified:   indent/sh.vim
> > #       modified:   plugin/utl.vim
> > #       modified:   snippets/_.snippets
> > #       modified:   syntax/elisp.vim
> > #       modified:   syntax/sh.vim
> > #       modified:   syntax/vim.vim
> >
> > `git commit <tab>' only suggests:
> > doc           filetype.vim  ftplugin      indent        plugin        snippets      syntax
> 
> Why’s that wrong?  Completing git commit syntax hopefully adds a slash
> and lets you complete the files in the sub-folder, right?

    Yes, you're right, even though I still prefer when `git ci <tab>' shows
and completes all the files immediately, not forcing me into first
deleting the completed slash and then continue completing the files in
the directory... But I guess Alexey's version might actually be more
consistent with Zsh's behaviour in that regard in other similar
situations. So, sorry for that point.

    Unfortunately, there seem to be other problems:
When I enter the doc directory in the above structure, `git commit
<tab>' only shows:

.. tags

instead of all the other changed files/directories (displayed relative
to the current subdirectory). Wasn't exactly that the problem we are
solving here?


    Štěpán

P.S.: Well now I think about it, of course you can just continue to
"manually" complete the `..', so in my case I have to first complete
../, then delete and recomplete the '/' or add and delete a space, then
continue similarly to get to the other files. Maybe it is the behaviour
you expect and like, but for me it's really suboptimal. Maybe it's just
some problem in my setup? Anyway, with my solution, git ci <tab> always
completes and shows all the files relative to the current directory,
i.e. in the form git-commit will accept, *immediately*. I still very
much prefer that behaviour.

Re: [PATCH] _git: offer files relative to current directory

[Mikael Magnusson]

2009/12/8 Štěpán Němec <stepnem@gmail.com>:
>    Yes, you're right, even though I still prefer when `git ci <tab>' shows
> and completes all the files immediately, not forcing me into first
> deleting the completed slash and then continue completing the files in
> the directory...

Just type a / and continue tabcompleting, this is exactly how it works
when you tabcomplete files normally.

-- 
Mikael Magnusson

Re: [PATCH] _git: offer files relative to current directory

[Alexey I. Froloff]

[-- Attachment #1: Type: text/plain, Size: 962 bytes --]

On Tue, Dec 08, 2009 at 11:48:39AM +0100, ??t??p??n N??mec wrote:
> In a repository where `git status' reports the following changed files:
> #       modified:   doc/tags
> #       modified:   filetype.vim
> #       modified:   ftplugin/vim.vim
> #       modified:   ftplugin/zsh.vim
> #       modified:   indent/sh.vim
> #       modified:   plugin/utl.vim
> #       modified:   snippets/_.snippets
> #       modified:   syntax/elisp.vim
> #       modified:   syntax/sh.vim
> #       modified:   syntax/vim.vim
> `git commit <tab>' only suggests:
> doc           filetype.vim  ftplugin      indent        plugin        snippets      syntax
> which is obviously wrong; so my current solution is still functionally
> better :-/
And what exactly wrong with that?  doc, ftplugin, indent, plugin,
snippets and syntax are directories, this is how _multi_parts
works.  Descend into doc and you'll see "only" .. and tags.

-- 
Regards,
Sir Raorn.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] _git: offer files relative to current directory

[Štěpán Němec]

On Tue, Dec 08, 2009 at 02:22:32PM +0300, Alexey I. Froloff wrote:
> On Tue, Dec 08, 2009 at 11:48:39AM +0100, ??t??p??n N??mec wrote:
> > In a repository where `git status' reports the following changed files:
> > #       modified:   doc/tags
> > #       modified:   filetype.vim
> > #       modified:   ftplugin/vim.vim
> > #       modified:   ftplugin/zsh.vim
> > #       modified:   indent/sh.vim
> > #       modified:   plugin/utl.vim
> > #       modified:   snippets/_.snippets
> > #       modified:   syntax/elisp.vim
> > #       modified:   syntax/sh.vim
> > #       modified:   syntax/vim.vim
> > `git commit <tab>' only suggests:
> > doc           filetype.vim  ftplugin      indent        plugin        snippets      syntax
> > which is obviously wrong; so my current solution is still functionally
> > better :-/
> And what exactly wrong with that?  doc, ftplugin, indent, plugin,
> snippets and syntax are directories, this is how _multi_parts
> works.  Descend into doc and you'll see "only" .. and tags.
> 
> -- 
> Regards,
> Sir Raorn.

Yeah, exactly, I just replied to the previous mail about that :-)

Well... so is there some way I can get the behaviour I want with your
solution?

    Štěpán

Re: [PATCH] _git: offer files relative to current directory

[Alexey I. Froloff]

[-- Attachment #1: Type: text/plain, Size: 704 bytes --]

On Tue, Dec 08, 2009 at 12:39:44PM +0100, ??t??p??n N??mec wrote:
> Well... so is there some way I can get the behaviour I want with your
> solution?
I don't understand what exactly do you want.  Original code
offers changed files relative to repository root.  Other files
offered relative to current directory.

My path does two things.  First, it makes "changed files" paths
to be relative to current directory, so git-commit accepts these
paths.  Second, it allows to complete paths outside of current
directory, which current code does not allow.

After my "relativisation" generated paths are split at '/' by
_multi_parts, I haven't changed this behavior.

-- 
Regards,
Sir Raorn.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] _git: offer files relative to current directory

[Alexey I. Froloff]

[-- Attachment #1: Type: text/plain, Size: 301 bytes --]

On Mon, Dec 07, 2009 at 12:52:33AM +0300, Alexey I. Froloff wrote:
> When offering repository files (cached, deleted, changed, etc), make
> sure that paths are relative to current directory, as git expects.
So, what about this patch?  Will it be accepted or rejected?

-- 
Regards,
Sir Raorn.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] _git: offer files relative to current directory

[Peter Stephenson]

On Sun, 13 Dec 2009 03:08:20 +0300
"Alexey I. Froloff" <raorn@altlinux.org> wrote:
> On Mon, Dec 07, 2009 at 12:52:33AM +0300, Alexey I. Froloff wrote:
> > When offering repository files (cached, deleted, changed, etc), make
> > sure that paths are relative to current directory, as git expects.
> So, what about this patch?  Will it be accepted or rejected?

Please can someone who uses git decide.

Thanks
pws

-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/

Re: [PATCH] _git: offer files relative to current directory

[Nikolai Weibull]

On Sun, Dec 13, 2009 at 22:33, Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:

> On Sun, 13 Dec 2009 03:08:20 +0300
> "Alexey I. Froloff" <raorn@altlinux.org> wrote:

>> On Mon, Dec 07, 2009 at 12:52:33AM +0300, Alexey I. Froloff wrote:

>> > When offering repository files (cached, deleted, changed, etc), make
>> > sure that paths are relative to current directory, as git expects.
>> So, what about this patch?  Will it be accepted or rejected?

> Please can someone who uses git decide.

I think that it should be included.  I'm the original author of _git,
by they way.

Re: [PATCH] _git: offer files relative to current directory

[Štěpán Němec]

On Sun, Dec 13, 2009 at 09:33:34PM +0000, Peter Stephenson wrote:
> On Sun, 13 Dec 2009 03:08:20 +0300
> "Alexey I. Froloff" <raorn@altlinux.org> wrote:
> > On Mon, Dec 07, 2009 at 12:52:33AM +0300, Alexey I. Froloff wrote:
> > > When offering repository files (cached, deleted, changed, etc), make
> > > sure that paths are relative to current directory, as git expects.
> > So, what about this patch?  Will it be accepted or rejected?
> 
> Please can someone who uses git decide.

I assume this calls for voices more authoritative than mine;
nevertheless, as someone who took part in its discussion and tested it,
I can only recommend this patch -- it fixes a real and visible problem
with the current git completion.

(I also remember a user complaint about the current incorrect behaviour
in the #zsh channel on Freenode -- that was actually what instigated my
interest in fixing the problem myself originally)

    Štěpán

Re: [PATCH] _git: offer files relative to current directory

[Jörg Sommer]

Hi Peter,

Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> On Sun, 13 Dec 2009 03:08:20 +0300
> "Alexey I. Froloff" <raorn@altlinux.org> wrote:
>> On Mon, Dec 07, 2009 at 12:52:33AM +0300, Alexey I. Froloff wrote:
>> > When offering repository files (cached, deleted, changed, etc), make
>> > sure that paths are relative to current directory, as git expects.
>> So, what about this patch?  Will it be accepted or rejected?
>
> Please can someone who uses git decide.

I've did some tests and this functions seems correct. But I would
refuse it for these reasons:

• The function can be written more self‐contained and generic if the
  prefix would be passed as argument and not determined internally.

• The intention of the function is the make paths relative to a given
  prefix. It should not beautify paths or something. Otherwise name it
  __git_files_relative_and_beatify.

• The function should not take and return a nul‐separated string but an
  array. The code looks as follows:
  call git
  call __git_files_relative
    split output of git
    …
    join output
  split output of __git_files_relative

Here's my test:

#v+
#!/bin/zsh

emulate -R zsh

_call_program () {
    local tmp
    if zstyle -s ":completion:${curcontext}:${1}" command tmp
    then
        if [[ "$tmp" = -* ]]
        then
            eval "$tmp[2,-1]" "$argv[2,-1]"
        else
            eval "$tmp"
        fi
    else
        eval "$argv[2,-1]"
    fi
}
__git_command_successful() { true; }

__git_files_relative () {
  local rawfiles files file f_parts prefix p_parts tmp

  prefix=$(_call_program gitprefix git rev-parse --show-prefix 2>/dev/null)
  __git_command_successful || return

  # Empty prefix, no modifications
  if (( $#prefix == 0 )); then
    print $1
    return
  fi

  rawfiles=(${(ps:\0:)1})
  files=()

  # Now we assume that we've given "absolute" paths list with "root"
  # being repository top directory.  $prefix is also "absolute" path.
  for file in $rawfiles; do
    # Collapse "/./" and "//", strip "/." and "/" from tail (I know,
    # this is a bit paranoid).
    f_parts=(${(s:/:)"${${${${file//\/\///}//\/.\///}%%/.}%%/}"})
    p_parts=(${(s:/:)"${${${${prefix//\/\///}//\/.\///}%%/.}%%/}"})
    tmp=()

    # Strip common path prefix.
    while (( $#f_parts > 0 )) && (( $#p_parts > 0 )) && [[ $f_parts[1] == $p_parts[1] ]]; do
      f_parts[1]=()
      p_parts[1]=()
    done

    # If prefix still not empty, ascend up.
    while (( $#p_parts > 0 )); do
	tmp+=..
	p_parts[1]=()
    done

    # Add remaining path.
    tmp=("$tmp[@]" "$f_parts[@]")

    files+=${(j:/:)tmp}
  done

  print ${(pj:\0:)files}
}

do_test()
{
    local no prefix arg expect
    no=$1
    prefix=$2
    arg=${3//$'\n'/$'\0'}
    expect=${4//$'\n'/$'\0'}

    zstyle ':completion:*:git:gitprefix' command "echo $prefix"
    outp=$(curcontext='git-test:git' __git_files_relative $arg)
    if [[ $outp != $expect ]]
    then
        print "test $no failed"
        print "output:"
        print ${outp//$'\0'/$'\n'}
        print "expected:"
        print ${expect//$'\0'/$'\n'}
        print '===================='
    fi
}

do_test 1 '' "$(print -l 'a/x' 'b')" "$(print -l 'a/x' 'b')"
do_test 2 'a' "$(print -l 'a/x' 'b')" "$(print -l 'x' '../b')"
do_test 3 'b' "$(print -l 'a/x' 'b/y')" "$(print -l '../a/x' 'y')"

do_test 4 'c' "$(print -l a/{1..3} b c/{m/{1..2},n/1,o})" \
  "$(print -l ../a/{1..3} ../b {m/{1..2},n/1,o})"

do_test 5 'c/n' "$(print -l a/{1..3} b c/{m/{1..2},n/1,o})" \
  "$(print -l ../../a/{1..3} ../../b {../m/{1..2},1,../o})"

do_test 6 'a' "$(print -l a/1 b/t/u/../v)" \
  "$(print -l 1 ../b/t/u/../v)"

do_test 7 'a' "$(print -l a/1 b/t/./v)" \
  "$(print -l 1 ../b/t/./v)"

do_test 8 'a' "$(print -l a/1 b/t/.///)" \
  "$(print -l 1 ../b/t/.///)"
#v-

Bye, Jörg.
-- 
Eine Blume geht über eine Wiese, sieht einen schönen Menschen und reißt
ihm den Kopf ab.

Re: [PATCH] _git: offer changed files relative to current directory

[Alexey I. Froloff]

[-- Attachment #1: Type: text/plain, Size: 316 bytes --]

On Sat, Dec 05, 2009 at 06:00:53PM +0100, ??t??p??n N??mec wrote:
> -  _wanted files expl 'index file' _multi_parts $@ - / files
> +  _description files expl 'Changed files'
> +  compadd "$expl[@]" - "$files[@]"
You don't use _multi_parts, what's why you think my code is
"wrong".

-- 
Regards,
Sir Raorn.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] _git: offer changed files relative to current directory

[Štěpán Němec]

On Tue, Dec 08, 2009 at 03:08:49PM +0300, Alexey I. Froloff wrote:
> On Sat, Dec 05, 2009 at 06:00:53PM +0100, ??t??p??n N??mec wrote:
> > -  _wanted files expl 'index file' _multi_parts $@ - / files
> > +  _description files expl 'Changed files'
> > +  compadd "$expl[@]" - "$files[@]"
> You don't use _multi_parts, what's why you think my code is
> "wrong".
Yeah, I figured as much; it's "wrong" only with respect to my
expectations, of course -- I also noted already that your solution
actually seems to be more consistent with other Zsh behaviour.

My solution actually duplicates the output of `git status', so it
displays and completes the whole paths immediately, at any position
within the repository. I still think it's (more) convenient and I kind
of got used to it, but the code is ridiculous and I assume others prefer
the behaviour of your solution, too.  

Thanks to all for input, also to Mikael for the one-keystroke-saving
suggestion ;-).

Regards,

    Štěpán

Re: [PATCH] _git: offer changed files relative to current directory

[Alexey I. Froloff]

[-- Attachment #1: Type: text/plain, Size: 551 bytes --]

On Tue, Dec 08, 2009 at 02:20:10PM +0100, ??t??p??n N??mec wrote:
> My solution actually duplicates the output of `git status', so it
> displays and completes the whole paths immediately, at any position
> within the repository. I still think it's (more) convenient and I kind
> of got used to it, but the code is ridiculous and I assume others prefer
> the behaviour of your solution, too.  
This could be easily zstyle'd, but in some cases it will be
annoying, esp. for __git_cached_files on large repositories.

-- 
Regards,
Sir Raorn.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]