RE: [PATCH] Completion must sort before using uniq

RE: [PATCH] Completion must sort before using uniq

[Joachim Schmitz]

Re-adding git@vger...

> From: Marc Khouzam [mailto:marc.khouzam@gmail.com]
> Sent: Friday, November 23, 2012 11:51 AM
> To: Joachim Schmitz
> Cc: szeder@ira.uka.de; felipe.contreras@gmail.com
> Subject: Re: [PATCH] Completion must sort before using uniq
> 
> On Fri, Nov 23, 2012 at 3:10 AM, Joachim Schmitz
> <jojo@schmitz-digital.de> wrote:
> > Marc Khouzam wrote:
> >> The uniq program only works with sorted input.  The man page states
> >> "uniq prints the unique lines in a sorted file".
> > ...
> >> --- a/contrib/completion/git-completion.bash
> >> +++ b/contrib/completion/git-completion.bash
> >> @@ -321,7 +321,7 @@ __git_refs ()
> >>                                if [[ "$ref" == "$cur"* ]]; then
> >>                                        echo "$ref"
> >>                                fi
> >> -                       done | uniq -u
> >> +                       done | sort | uniq -u
> >
> > Is 'sort -u' not universally available and sufficient here? It is POSIX
> > at least:
> > http://pubs.opengroup.org/onlinepubs/9699919799/utilities/sort.html
> 
> "-u Unique: suppress all but one in each set of lines having equal
> keys. If used with the -c option, check that there are no lines with
> duplicate keys, in addition to checking that the input file is
> sorted."
> 
> What the code aims to do is to only show lines that are not
> duplicated.  'sort -u' would still output one line for each duplicated
> one.  It seems 'sort -u' is the equivalent of 'sort | uniq' but won't
> replace 'sort | uniq -u'.

I can't see the difference and in fact don't understand uniq's -u option al all
Linux man pages say: "only print unique lines", but that is what uniq does by default anyway?!?

> Is 'sort | uniq -u' not POSIX?

It is. It is one process more though.

Bye, Jojo

Re: [PATCH] Completion must sort before using uniq

[Sascha Cunz]

> I can't see the difference and in fact don't understand uniq's -u option al
> all Linux man pages say: "only print unique lines", but that is what uniq
> does by default anyway?!?

>From the german translation of uniq's man-page, you can deduct that "only 
print unique lines" actually means: "print lines that are _not repeated_ in 
the input".

A short test confirms that. i.e.:

	printf "a\nb\nb\nc\n" | uniq -u

gives:
	a
	c

Sascha

RE: [PATCH] Completion must sort before using uniq

[Joachim Schmitz]

> From: Sascha Cunz [mailto:sascha-ml@babbelbox.org]
> Sent: Friday, November 23, 2012 1:26 PM
> To: Joachim Schmitz
> Cc: 'Marc Khouzam'; git@vger.kernel.org; szeder@ira.uka.de; felipe.contreras@gmail.com
> Subject: Re: [PATCH] Completion must sort before using uniq
> 
> > I can't see the difference and in fact don't understand uniq's -u option al
> > all Linux man pages say: "only print unique lines", but that is what uniq
> > does by default anyway?!?
> 
> From the german translation of uniq's man-page, you can deduct that "only
> print unique lines" actually means: "print lines that are _not repeated_ in
> the input".
> 
> A short test confirms that. i.e.:
> 
> 	printf "a\nb\nb\nc\n" | uniq -u
> 
> gives:
> 	a
> 	c

Ah, OK, then I rest my case. Sorry for the noise.

[PATCH] Completion must sort before using uniq

[Marc Khouzam]

The uniq program only works with sorted input.  The man page states
"uniq prints the unique lines in a sorted file".

When __git_refs use the guess heuristic employed by checkout for
tracking branches it wants to consider remote branches but only if
the branch name is unique.  To do that, it calls 'uniq -u'.  However
the input given to 'uniq -u' is not sorted.

For example if all available branches are:
  master
  remotes/GitHub/maint
  remotes/GitHub/master
  remotes/origin/maint
  remotes/origin/master

When performing completion on 'git checkout ma' the choices given are
  maint
  master
but when performing completion on 'git checkout mai', no choices
appear, which is obviously contradictory.

The reason is that, when dealing with 'git checkout ma',
"__git_refs '' 1" will find the following list:
  master
  maint
  master
  maint
  master
which, when passed to 'uniq -u' will remain the same.
But when dealing with 'git checkout mai', the list will be:
  maint
  maint
which happens to be sorted and will be emptied by 'uniq -u'.

The solution is to first call 'sort' and then 'uniq -u'.

Signed-off-by: Marc Khouzam <marc.khouzam@gmail.com>
---

I ran into this by fluke when testing the tcsh completion.

Thanks for considering the fix.

Marc

 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index bc0657a..85ae419 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -321,7 +321,7 @@ __git_refs ()
                                if [[ "$ref" == "$cur"* ]]; then
                                        echo "$ref"
                                fi
-                       done | uniq -u
+                       done | sort | uniq -u
                fi
                return
        fi
--
1.8.0.1.g9fe2839

Re: [PATCH] Completion must sort before using uniq

[Joachim Schmitz]

Marc Khouzam wrote:
> The uniq program only works with sorted input.  The man page states
> "uniq prints the unique lines in a sorted file".
...
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -321,7 +321,7 @@ __git_refs ()
>                                if [[ "$ref" == "$cur"* ]]; then
>                                        echo "$ref"
>                                fi
> -                       done | uniq -u
> +                       done | sort | uniq -u

Is 'sort -u' not universally available and sufficient here? It is POSIX at 
least:
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/sort.html

Bye, Jojo 

Re: [PATCH] Completion must sort before using uniq

[Felipe Contreras]

On Thu, Nov 22, 2012 at 5:16 AM, Marc Khouzam <marc.khouzam@gmail.com> wrote:
> The uniq program only works with sorted input.  The man page states
> "uniq prints the unique lines in a sorted file".
>
> When __git_refs use the guess heuristic employed by checkout for
> tracking branches it wants to consider remote branches but only if
> the branch name is unique.  To do that, it calls 'uniq -u'.  However
> the input given to 'uniq -u' is not sorted.
>
> For example if all available branches are:
>   master
>   remotes/GitHub/maint
>   remotes/GitHub/master
>   remotes/origin/maint
>   remotes/origin/master
>
> When performing completion on 'git checkout ma' the choices given are
>   maint
>   master
> but when performing completion on 'git checkout mai', no choices
> appear, which is obviously contradictory.
>
> The reason is that, when dealing with 'git checkout ma',
> "__git_refs '' 1" will find the following list:
>   master
>   maint
>   master
>   maint
>   master
> which, when passed to 'uniq -u' will remain the same.
> But when dealing with 'git checkout mai', the list will be:
>   maint
>   maint
> which happens to be sorted and will be emptied by 'uniq -u'.
>
> The solution is to first call 'sort' and then 'uniq -u'.

The solution to what? This seems to be the right thing indeed, but you
don't explain what is the actual problem that is being solved. What
does the user experience? What would (s)he experience after the patch?

-- 
Felipe Contreras