Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Simple update to bash completions to prevent unbound variable errors.
From: Ted Pavlic @ 2009-01-13  4:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Shawn O. Pearce, git
In-Reply-To: <7vy6xfew2n.fsf@gitster.siamese.dyndns.org>

>> A vim modeline has also been added for consistency.
>
> Yuck.

Better that than have a mixture of spaces and tabs.

>> +# __git_ps1 accepts 0 or 1 arguments (i.e., format string)
>> +# returns text to add to bash PS1 prompt (includes branch name)
>
> Good.  Would be better if you described what $1 and $2 mean.

In that case, there's only $1 (the format string).

Note that in most cases, I didn't know what $1 and $2 were. I was just 
trying to fix it so that it would work on my system.

>> -		if [ -n "$1" ]; then
>> +		if [ $# -gt 0 ]&&  [ -n "$1" ]; then
>
> I found the previous round's [ -n "${1-}" ] much easier to read, if we were to
> do this.  If -n "${1-}", then "$1" is definitely set so nothing need to
> change in the then ... else part.

Hey -- I agree, but no one else liked ${1-}. And hg's bash completion 
seems far superior because they avoid ever having to worry about it. 
They actually thought about the arguments ahead of time.

>> +# __gitcomp_1 requires 2 arguments
>
> ... and $1 and $2 mean?

No clue. Patches are welcome.

> Yuck.  If you are taking advantage of the fact that "local one"
> will bind one to emptiness anyway, can't you do something like:
>
> 	local one=${1-} two=${2-} cur=${3-} four=${4-}

Why even use one, two three, and four then?

I had ${#-} throughout, but I was told that that was ugly. So the best I 
could do was come up with the above mess.

--Ted

-- 
Ted Pavlic <ted@tedpavlic.com>

   Please visit my ALS association page:
         http://web.alsa.org/goto/tedpavlic
   My family appreciates your support in the fight to defeat ALS.

^ permalink raw reply

* Re: [PATCH] Simple update to bash completions to prevent unbound variable errors.
From: Boyd Stephen Smith Jr. @ 2009-01-13  3:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ted Pavlic, Shawn O. Pearce, git
In-Reply-To: <7vy6xfew2n.fsf@gitster.siamese.dyndns.org>

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

On Monday 2009 January 12 21:14:40 Junio C Hamano wrote:
>Ted Pavlic <ted@tedpavlic.com> writes:
>> A vim modeline has also been added for consistency.
>
>Yuck.

While I dislike emacs and use vim pretty much exclusively, I don't really see 
the need for a vim modeline.  On top of that, fdm=marker is a bit silly since 
there aren't any markers.  ff=unix and ft=sh are redundant, as any vim should 
detect these properly, given the filename.

So, I'm slightly negative on the modeline hunk.

>> @@ -111,7 +115,7 @@ __git_ps1 ()
>>  			fi
>>  		fi
>>
>> -		if [ -n "$1" ]; then
>> +		if [ $# -gt 0 ] && [ -n "$1" ]; then
>
>I found the previous round's [ -n "${1-}" ] much easier to read, if we were
> to do this.  If -n "${1-}", then "$1" is definitely set so nothing need to
> change in the then ... else part.

I found "${1-}" ugly, and this a bit better, but I'll defer to Junio.

>> @@ -131,11 +136,22 @@ __gitcomp_1 ()
>>  	done
>>  }
>>
>> +# __gitcomp accepts 1, 2, 3, or 4 arguments
>> +# generates completion reply with compgen
>>  __gitcomp ()
>>  {
>> -	local cur="${COMP_WORDS[COMP_CWORD]}"
>> -	if [ $# -gt 2 ]; then
>> -		cur="$3"
>> +	local one two cur="${COMP_WORDS[COMP_CWORD]}" four
>> +	if [ $# -gt 0 ]; then
>> +		one="$1"
>> +		if [ $# -gt 1 ]; then
>> +			two="$2"
>> +			if [ $# -gt 2 ]; then
>> +				cur="$3"
>> +				if [ $# -gt 3 ]; then
>> +					four="$4"
>> +				fi
>> +			fi
>> +		fi
>>  	fi
>
>Yuck.

Definitely agreeing with Junio here.  This is far less ascetic than the old 
patch.  Truth be told, this whole thread would probably have been more 
productive without me chiming in.  Sorry about that.

>If you are taking advantage of the fact that "local one" 
>will bind one to emptiness anyway, can't you do something like:
>
>	local one=${1-} two=${2-} cur=${3-} four=${4-}

Even better to use variable names that match the usage, if possible. 
-- 
Boyd Stephen Smith Jr.                     ,= ,-_-. =. 
bss@iguanasuicide.net                     ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy           `-'(. .)`-' 
http://iguanasuicide.net/                      \_/     

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: the meaning of keephardlinks
From: Geoff Russell @ 2009-01-13  3:30 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0901130141190.3586@pacific.mpi-cbg.de>

On 1/13/09, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
>
>  On Tue, 13 Jan 2009, Geoff Russell wrote:
>
>  > I'm curious about what keephardlinks means.
>  >
>  > If I do: "ln X Y ; git add Y ; git commit" in my origin and then
>  > "git pull" in my cloned repository,
>  > should I get a hard linked file in the clone
>  > when core.keephardlinks is set to true?
>
>
> Nope.
>
>  It means that if you have a hard link locally, it will stay a hard link
>  (and if it is modified, the other linked files will obviously change,
>  too).
>
>  Note that this feature is not even in 'next'.
>
>  Ciao,
>  Dscho


Fine. Thanks,

Geoff.

^ permalink raw reply

* [PATCH/RFC] Documentation/SubmittingPatches: emacs whitespace tip
From: jidanni @ 2009-01-13  3:27 UTC (permalink / raw)
  To: git

Signed-off-by: jidanni <jidanni@jidanni.org>
---
 Documentation/SubmittingPatches |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index ba07c8c..7b64343 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -442,6 +442,8 @@ whitespaces (fatal in patches).  Running 'C-u g' to display the
 message in raw form before using '|' to run the pipe can work
 this problem around.
 
+Inside gnus or not, a (setq-default show-trailing-whitespace t) in
+one's ~/.emacs file will reduce whitespace embarrassment incidents.
 
 KMail
 -----
-- 
1.6.0.6

^ permalink raw reply related

* Re: [PATCH] Simple update to bash completions to prevent unbound variable errors.
From: Junio C Hamano @ 2009-01-13  3:14 UTC (permalink / raw)
  To: Ted Pavlic; +Cc: Shawn O. Pearce, git
In-Reply-To: <496C0003.7040909@tedpavlic.com>

Ted Pavlic <ted@tedpavlic.com> writes:

> A vim modeline has also been added for consistency.

Yuck.

> Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
> ---
>  contrib/completion/git-completion.bash |   70
> +++++++++++++++++++++++++-------
>  1 files changed, 55 insertions(+), 15 deletions(-)

Double yuck.

> diff --git a/contrib/completion/git-completion.bash
> b/contrib/completion/git-completion.bash
> index 7b074d7..619e886 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -50,9 +50,11 @@ case "$COMP_WORDBREAKS" in
>  *)   COMP_WORDBREAKS="$COMP_WORDBREAKS:"
>  esac
>
> +# __gitdir accepts 0 or 1 arguments (i.e., location)
> +# returns location of .git repo
>  __gitdir ()
>  {
> -	if [ -z "$1" ]; then
> +	if [ $# -eq 0 ] || [ -z "$1" ]; then
>  		if [ -n "$__git_dir" ]; then
>  			echo "$__git_dir"
>  		elif [ -d .git ]; then
> @@ -67,6 +69,8 @@ __gitdir ()
>  	fi
>  }
>
> +# __git_ps1 accepts 0 or 1 arguments (i.e., format string)
> +# returns text to add to bash PS1 prompt (includes branch name)

Good.  Would be better if you described what $1 and $2 mean.

> @@ -111,7 +115,7 @@ __git_ps1 ()
>  			fi
>  		fi
>
> -		if [ -n "$1" ]; then
> +		if [ $# -gt 0 ] && [ -n "$1" ]; then

I found the previous round's [ -n "${1-}" ] much easier to read, if we were to
do this.  If -n "${1-}", then "$1" is definitely set so nothing need to
change in the then ... else part.

> +# __gitcomp_1 requires 2 arguments

... and $1 and $2 mean?

>  __gitcomp_1 ()
>  {
>  	local c IFS=' '$'\t'$'\n'
> @@ -131,11 +136,22 @@ __gitcomp_1 ()
>  	done
>  }
>
> +# __gitcomp accepts 1, 2, 3, or 4 arguments
> +# generates completion reply with compgen
>  __gitcomp ()
>  {
> -	local cur="${COMP_WORDS[COMP_CWORD]}"
> -	if [ $# -gt 2 ]; then
> -		cur="$3"
> +	local one two cur="${COMP_WORDS[COMP_CWORD]}" four
> +	if [ $# -gt 0 ]; then
> +		one="$1"
> +		if [ $# -gt 1 ]; then
> +			two="$2"
> +			if [ $# -gt 2 ]; then
> +				cur="$3"
> +				if [ $# -gt 3 ]; then
> +					four="$4"
> +				fi
> +			fi
> +		fi
>  	fi

Yuck.  If you are taking advantage of the fact that "local one"
will bind one to emptiness anyway, can't you do something like:

	local one=${1-} two=${2-} cur=${3-} four=${4-}

to avoid this ugliness?

^ permalink raw reply

* log-tree.c: date hardwired
From: jidanni @ 2009-01-13  3:13 UTC (permalink / raw)
  To: git

In log-tree.c: printf("From %s Mon Sep 17 00:00:00 2001\n", name);
Wouldn't it be more aesthetically pleasing to use current local or UTC time?
Or at least comment in the code that the date is hardwired like that
in the fear that otherwise people will think it is the actual commit time.
No, I can't think of any other tool that hardwires the From separators
they produce.

^ permalink raw reply

* [PATCH] Simple update to bash completions to prevent unbound variable errors.
From: Ted Pavlic @ 2009-01-13  2:44 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, gitster

Below is a cleaner patch that prevents problems with unbound arguments
when bash is being operated under "set -u."

Some comments were added to remind callers about the requirements of
each __git* utility function.

Additionally, the completion call to bash has been modernized a bit.

A vim modeline has also been added for consistency.

Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
---
  contrib/completion/git-completion.bash |   70 
+++++++++++++++++++++++++-------
  1 files changed, 55 insertions(+), 15 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 7b074d7..619e886 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -50,9 +50,11 @@ case "$COMP_WORDBREAKS" in
  *)   COMP_WORDBREAKS="$COMP_WORDBREAKS:"
  esac

+# __gitdir accepts 0 or 1 arguments (i.e., location)
+# returns location of .git repo
  __gitdir ()
  {
-	if [ -z "$1" ]; then
+	if [ $# -eq 0 ] || [ -z "$1" ]; then
  		if [ -n "$__git_dir" ]; then
  			echo "$__git_dir"
  		elif [ -d .git ]; then
@@ -67,6 +69,8 @@ __gitdir ()
  	fi
  }

+# __git_ps1 accepts 0 or 1 arguments (i.e., format string)
+# returns text to add to bash PS1 prompt (includes branch name)
  __git_ps1 ()
  {
  	local g="$(git rev-parse --git-dir 2>/dev/null)"
@@ -111,7 +115,7 @@ __git_ps1 ()
  			fi
  		fi

-		if [ -n "$1" ]; then
+		if [ $# -gt 0 ] && [ -n "$1" ]; then
  			printf "$1" "${b##refs/heads/}$r"
  		else
  			printf " (%s)" "${b##refs/heads/}$r"
@@ -119,6 +123,7 @@ __git_ps1 ()
  	fi
  }

+# __gitcomp_1 requires 2 arguments
  __gitcomp_1 ()
  {
  	local c IFS=' '$'\t'$'\n'
@@ -131,11 +136,22 @@ __gitcomp_1 ()
  	done
  }

+# __gitcomp accepts 1, 2, 3, or 4 arguments
+# generates completion reply with compgen
  __gitcomp ()
  {
-	local cur="${COMP_WORDS[COMP_CWORD]}"
-	if [ $# -gt 2 ]; then
-		cur="$3"
+	local one two cur="${COMP_WORDS[COMP_CWORD]}" four
+	if [ $# -gt 0 ]; then
+		one="$1"
+		if [ $# -gt 1 ]; then
+			two="$2"
+			if [ $# -gt 2 ]; then
+				cur="$3"
+				if [ $# -gt 3 ]; then
+					four="$4"
+				fi
+			fi
+		fi
  	fi
  	case "$cur" in
  	--*=)
@@ -143,22 +159,27 @@ __gitcomp ()
  		;;
  	*)
  		local IFS=$'\n'
-		COMPREPLY=($(compgen -P "$2" \
-			-W "$(__gitcomp_1 "$1" "$4")" \
+		COMPREPLY=($(compgen -P "$two" \
+			-W "$(__gitcomp_1 "$one" "$four")" \
  			-- "$cur"))
  		;;
  	esac
  }

+# __git_heads accepts 0 or 1 arguments (to pass to __gitdir)
  __git_heads ()
  {
-	local cmd i is_hash=y dir="$(__gitdir "$1")"
+	local one
+	if [ $# -gt 0 ]; then
+		one="$1"
+	fi
+	local cmd i is_hash=y dir="$(__gitdir "$one")"
  	if [ -d "$dir" ]; then
  		git --git-dir="$dir" for-each-ref --format='%(refname:short)' \
  			refs/heads
  		return
  	fi
-	for i in $(git ls-remote "$1" 2>/dev/null); do
+	for i in $(git ls-remote "$one" 2>/dev/null); do
  		case "$is_hash,$i" in
  		y,*) is_hash=n ;;
  		n,*^{}) is_hash=y ;;
@@ -168,15 +189,20 @@ __git_heads ()
  	done
  }

+# __git_tags accepts 0 or 1 arguments (to pass to __gitdir)
  __git_tags ()
  {
-	local cmd i is_hash=y dir="$(__gitdir "$1")"
+	local one
+	if [ $# -gt 0 ]; then
+		one="$1"
+	fi
+	local cmd i is_hash=y dir="$(__gitdir "$one")"
  	if [ -d "$dir" ]; then
  		git --git-dir="$dir" for-each-ref --format='%(refname:short)' \
  			refs/tags
  		return
  	fi
-	for i in $(git ls-remote "$1" 2>/dev/null); do
+	for i in $(git ls-remote "$one" 2>/dev/null); do
  		case "$is_hash,$i" in
  		y,*) is_hash=n ;;
  		n,*^{}) is_hash=y ;;
@@ -186,9 +212,14 @@ __git_tags ()
  	done
  }

+# __git_refs accepts 0 or 1 arguments (to pass to __gitdir)
  __git_refs ()
  {
-	local i is_hash=y dir="$(__gitdir "$1")"
+	local one
+	if [ $# -gt 0 ]; then
+		one="$1"
+	fi
+	local i is_hash=y dir="$(__gitdir "$one")"
  	local cur="${COMP_WORDS[COMP_CWORD]}" format refs
  	if [ -d "$dir" ]; then
  		case "$cur" in
@@ -218,6 +249,7 @@ __git_refs ()
  	done
  }

+# __git_refs2 requires 1 argument (to pass to __git_refs)
  __git_refs2 ()
  {
  	local i
@@ -226,6 +258,7 @@ __git_refs2 ()
  	done
  }

+# __git_refs_remotes requires 1 argument (to pass to ls-remote)
  __git_refs_remotes ()
  {
  	local cmd i is_hash=y
@@ -470,6 +503,7 @@ __git_aliases ()
  	done
  }

+# __git_aliased_command requires 1 argument
  __git_aliased_command ()
  {
  	local word cmdline=$(git --git-dir="$(__gitdir)" \
@@ -482,6 +516,7 @@ __git_aliased_command ()
  	done
  }

+# __git_find_subcommand requires 1 argument
  __git_find_subcommand ()
  {
  	local word subcommand c=1
@@ -1766,13 +1801,18 @@ _gitk ()
  	__git_complete_revlist
  }

-complete -o default -o nospace -F _git git
-complete -o default -o nospace -F _gitk gitk
+complete -o bashdefault -o default -o nospace -F _git git 2>/dev/null \
+	|| complete -o default -o nospace -F _git git
+complete -o bashdefault -o default -o nospace -F _gitk gitk 2>/dev/null \
+	|| complete -o default -o nospace -F _gitk gitk

  # The following are necessary only for Cygwin, and only are needed
  # when the user has tab-completed the executable name and consequently
  # included the '.exe' suffix.
  #
  if [ Cygwin = "$(uname -o 2>/dev/null)" ]; then
-complete -o default -o nospace -F _git git.exe
+complete -o bashdefault -o default -o nospace -F _git git.exe 2>/dev/null \
+	|| complete -o default -o nospace -F _git git.exe
  fi
+
+# vim:ft=sh:fdm=marker:ff=unix:noet:ts=4:sw=4
-- 
1.6.1.87.g15624

^ permalink raw reply related

* Re: git-gui: the UTF-8 characters are shown as garbage
From: jidanni @ 2009-01-13  2:24 UTC (permalink / raw)
  To: spearce; +Cc: 497469, git
In-Reply-To: <20080901221138.6458.4316.reportbug@ant.local>

As in http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=497469
I also see junk with e.g.,
$ git gui blame git-gui/po/zh_cn.po

^ permalink raw reply

* [PATCH] contrib/vim: change URL to point to the latest syntax files
From: Markus Heidelberg @ 2009-01-13  2:10 UTC (permalink / raw)
  To: Jeff King; +Cc: git

Vim's SVN repository doesn't offer the latest runtime files, since
normally they are only updated there on a release. Though currently
there is no difference between the SVN and HTTP/FTP version of the git
syntax files.

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---
 contrib/vim/README |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/contrib/vim/README b/contrib/vim/README
index c487346..fca1e17 100644
--- a/contrib/vim/README
+++ b/contrib/vim/README
@@ -5,11 +5,13 @@ automatically.
 If you have an older version of vim, you can get the latest syntax
 files from the vim project:
 
-  http://vim.svn.sourceforge.net/viewvc/vim/trunk/runtime/syntax/git.vim
-  http://vim.svn.sourceforge.net/viewvc/vim/trunk/runtime/syntax/gitcommit.vim
-  http://vim.svn.sourceforge.net/viewvc/vim/trunk/runtime/syntax/gitconfig.vim
-  http://vim.svn.sourceforge.net/viewvc/vim/trunk/runtime/syntax/gitrebase.vim
-  http://vim.svn.sourceforge.net/viewvc/vim/trunk/runtime/syntax/gitsendemail.vim
+  http://ftp.vim.org/pub/vim/runtime/syntax/git.vim
+  http://ftp.vim.org/pub/vim/runtime/syntax/gitcommit.vim
+  http://ftp.vim.org/pub/vim/runtime/syntax/gitconfig.vim
+  http://ftp.vim.org/pub/vim/runtime/syntax/gitrebase.vim
+  http://ftp.vim.org/pub/vim/runtime/syntax/gitsendemail.vim
+
+These files are also available via FTP at the same location.
 
 To install:
 
-- 
1.6.1.35.g0c23

^ permalink raw reply related

* Re: [PATCH/RFC] Documentation/git-blame.txt, git-gui.txt: link SEE ALSOs
From: jidanni @ 2009-01-13  2:05 UTC (permalink / raw)
  To: gitster; +Cc: spearce, git
In-Reply-To: <7v8wpgf04c.fsf@gitster.siamese.dyndns.org>

OK, JCH's (and Shawn's, if any forthcoming :-)) patches look better than mine.
Please apply them. Thanks.

^ permalink raw reply

* Re: [PATCH] Fix Dcoumentation typos surrounding the word 'handful'.
From: Sam Vilain @ 2009-01-13  2:02 UTC (permalink / raw)
  To: Jon Loeliger; +Cc: git
In-Reply-To: <1231790527-7515-1-git-send-email-jdl@freescale.com>

Jon Loeliger wrote:
> Subject: Fix Dcoumentation typos surrounding the word 'handful'.
>   

Do ironic subjects meet acceptance criteria? :)

Sam.

^ permalink raw reply

* Re: [PATCH] gitweb: recognize six digit abbreviated SHA1
From: Sam Vilain @ 2009-01-13  2:00 UTC (permalink / raw)
  To: Anders Melchiorsen; +Cc: git, jnareb, gitster
In-Reply-To: <87mydw2hkb.fsf@cup.kalibalik.dk>

Anders Melchiorsen wrote:
> Anders Melchiorsen <mail@cup.kalibalik.dk> writes:
>
>   
>> +	if ($line =~ m/([0-9a-fA-F]{7,40})/) {
>>     
>
> I could not make up my mind between the seven digits from "git
> rev-parse --short" and the six digits currently used by gitk.
>
> So I put one option in the patch, and the other one in the subject.
>   

I think if you're going to go so short as 6 digits, it's probably worth
making sure that the really short SHA1s check commits only. eg, if
you've got a commit 'fa023473' and a tree 'fa023421', then 'fa0234'
should match the commit and not the tree. But I don't think there's a
plumbing way to do a query like that at the moment.

Even git.git isn't unique over 6 digits, even restricting to commits.
Not since b0a3de ;-).

Sam

^ permalink raw reply

* Re: git/webdav is refusing to authenticate properly.
From: Boyd Stephen Smith Jr. @ 2009-01-13  1:53 UTC (permalink / raw)
  To: Peter Spierenburg; +Cc: git
In-Reply-To: <496BE1E0.9010908@gmail.com>

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

On Monday 12 January 2009, Peter Spierenburg 
<ionlyusethisaddressforlists@gmail.com> wrote about 'Re: git/webdav is 
refusing to authenticate properly.':
>C'mon, leave my password 'in-the-clear', in a text file on a networked
>box? That is the kind of prank a fourth-year student would try to pull
>on a freshman.
>
>How do I really do it?

AFAIK, that's the only way for now.

Personally, I'd welcome a patch that allowed fetch/push to prompt the user 
for a password, but I'm not holding my breath.  If I want to limit access 
to a few users, I serve that repository over ssh and depend on ssh for 
authentication and filesystem permissions for authorization.
-- 
Boyd Stephen Smith Jr.                     ,= ,-_-. =. 
bss@iguanasuicide.net                     ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy           `-'(. .)`-' 
http://iguanasuicide.net/                      \_/     

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCH,v2] contrib/examples/README: give an explanation of the status of these files
From: Junio C Hamano @ 2009-01-13  1:48 UTC (permalink / raw)
  To: jidanni; +Cc: git
In-Reply-To: <877i50ovdd.fsf_-_@jidanni.org>

Thanks.

^ permalink raw reply

* Re: [PATCH/RFC] Documentation/git-blame.txt, git-gui.txt: link SEE ALSOs
From: Junio C Hamano @ 2009-01-13  1:47 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, jidanni
In-Reply-To: <87bpucovnz.fsf@jidanni.org>

jidanni@jidanni.org writes:

> As git gui is heavily blame focused, we link its SEE ALSO to
> git-blame, and add a link back while we're at it.
>
> Signed-off-by: jidanni <jidanni@jidanni.org>
> ---
>  Documentation/git-blame.txt |    1 +
>  Documentation/git-gui.txt   |    3 +++
>  2 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
> index fba374d..d71a2c3 100644
> --- a/Documentation/git-blame.txt
> +++ b/Documentation/git-blame.txt
> @@ -186,6 +186,7 @@ commit commentary), a blame viewer won't ever care.
>  
>  SEE ALSO
>  --------
> +linkgit:git-gui[1],
>  linkgit:git-annotate[1]
>  
>  AUTHOR
> diff --git a/Documentation/git-gui.txt b/Documentation/git-gui.txt
> index d0bc98b..3a71074 100644
> --- a/Documentation/git-gui.txt
> +++ b/Documentation/git-gui.txt
> @@ -105,6 +105,9 @@ linkgit:gitk[1]::
>  	and file differences.  gitk is the utility started by
>  	'git-gui''s Repository Visualize actions.
>  
> +linkgit:git-blame[1]::
> +	Command-line blame viewer.
> +
>  Other
>  -----
>  'git-gui' is actually maintained as an independent project, but stable

As a general principle, I tend to refrain from referring to X from the
description of Y only because X happens to use Y heavily.

Referring people who heavily use Y to an alternative, which is X, hoping
that X may give a better user experience in certain environments is a
different matter, but in such a case, I'd rather see not just link but
in-text description as well (study the way "log -S" is suggested in the
description part for an example).  The attached patch shows you how.

On the other hand, what X does using Y sometimes may be easier to
understand if the reader is familiar with the way how Y works.  Even in
such a case, I think the documentation of X should be self contained
enough and ideally it shouldn't have to refer to Y.  And in the case of
git-gui documentation, I think it is.

So I am moderately negative about the first hunk of this patch as-is, and
I'll leave the decision on the second hunk to Shawn.



diff --git i/Documentation/git-blame.txt w/Documentation/git-blame.txt
index fba374d..ff7bbfb 100644
--- i/Documentation/git-blame.txt
+++ w/Documentation/git-blame.txt
@@ -36,6 +36,11 @@ $ git log --pretty=oneline -S'blame_usage'
 ea4c7f9bf69e781dd0cd88d2bccb2bf5cc15c9a7 git-blame: Make the output
 -----------------------------------------------------------------------------
 
+People working in GUI environment may find linkgit:git-gui[1] an useful
+alternative that provides an interactive interface to the history of
+each line.  It uses this command as an underlying engine.
+
+
 OPTIONS
 -------
 include::blame-options.txt[]

^ permalink raw reply related

* Re: [RFC/PATCH 3/7] replace_object: add mechanism to replace objects found in "refs/replace/"
From: Junio C Hamano @ 2009-01-13  1:31 UTC (permalink / raw)
  To: Christian Couder; +Cc: git
In-Reply-To: <20090112184403.ebb99b75.chriscool@tuxfamily.org>

Christian Couder <chriscool@tuxfamily.org> writes:

> diff --git a/replace_object.c b/replace_object.c
> new file mode 100644
> index 0000000..25b3ef3
> --- /dev/null
> +++ b/replace_object.c
> @@ -0,0 +1,116 @@
> +#include "cache.h"
> +#include "refs.h"
> +
> +static struct replace_object {
> +	unsigned char sha1[2][20];
> +} **replace_object;
> +
> +static int replace_object_alloc, replace_object_nr;
> +
> +static int replace_object_pos(const unsigned char *sha1)
> +{
> +	int lo, hi;
> +	lo = 0;
> +	hi = replace_object_nr;
> +	while (lo < hi) {
> +		int mi = (lo + hi) / 2;
> +		struct replace_object *rep = replace_object[mi];
> +		int cmp = hashcmp(sha1, rep->sha1[0]);
> +		if (!cmp)
> +			return mi;
> +		if (cmp < 0)
> +			hi = mi;
> +		else
> +			lo = mi + 1;
> +	}
> +	return -lo - 1;
> +}

Hmm, this is a tangent of this topic, but I wonder if we can do something
more generic to factor out many binary search like this we have throughout
the code.  Also I wonder if they can be made more efficient by taking
advantage of the fact that our hash is expected to produce a good uniform
distribution, similar to the way patch-ids.c::patch_pos() does this.

I guess the performance should not matter much for this table, as we
expect there won't be massive object replacements.

Also I recall Dscho muttered something about hashmap I didn't quite
understand.

> +static int register_replace_ref(const char *refname,
> +				const unsigned char *sha1,
> +				int flag, void *cb_data)
> +{
> +	/* Get sha1 from refname */
> +	const char *slash = strrchr(refname, '/');
> +	const char *hash = slash ? slash + 1 : refname;
> +	struct replace_object * repl_obj = xmalloc(sizeof(*repl_obj));

	struct replace_object *repl_obj = ...

> +	if (strlen(hash) != 40 || get_sha1_hex(hash, repl_obj->sha1[0])) {
> +		free(repl_obj);
> +		warning("bad replace ref name: %s", refname);
> +	}
> +
> +	/* Copy sha1 from the read ref */
> +	hashcpy(repl_obj->sha1[1], sha1);

Upon an error, you free and warn and then still copy into it?

> +	/* Register new object */
> +	if (register_replace_object(repl_obj, 1))
> +		warning("duplicate replace ref: %s", refname);

I'd say this is a grave error and should be reported as a repository
corruption.

> +static void prepare_replace_object(void)
> +{
> +	static int replace_object_prepared;
> +
> +	if (replace_object_prepared)
> +		return;
> +
> +	for_each_replace_ref(register_replace_ref, NULL);
> +	replace_object_prepared = 1;
> +}
> +
> +/* We allow "recursive" replacement. Only within reason, though */
> +#define MAXREPLACEDEPTH 5
> +
> +const unsigned char *lookup_replace_object(const unsigned char *sha1)
> +{
> +	int pos, depth = MAXREPLACEDEPTH;
> +	const unsigned char *cur = sha1;
> +
> +	prepare_replace_object();
> +
> +	/* Try to recursively replace the object */
> +	do {
> +		if (--depth < 0)
> +			die("replace depth too high for object %s",
> +			    sha1_to_hex(sha1));
> +
> +		pos = replace_object_pos(cur);
> +		if (0 <= pos)
> +			cur = replace_object[pos]->sha1[1];
> +	} while (0 <= pos);
> +
> +	return cur;
> +}

Since your paradigm is prepare replacement once at the beginning, never
allowing to update it, I think you can update the table while you look it
up.  E.g. if A->B and B->C exists, and A is asked for, you find A->B (to
tentatively make cur to point at B) and then you find B->C, and before
returning you can rewrite the first mapping to A->C.  Later look-up won't
need to dereference the table twice that way.

This assumes that there will be small number of replacements, but the same
object can be asked for more than once during the process.

> diff --git a/sha1_file.c b/sha1_file.c
> index f08493f..4f2fd10 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -2163,10 +2163,18 @@ static void *read_object(const unsigned char *sha1, enum object_type *type,
>  void *read_sha1_file(const unsigned char *sha1, enum object_type *type,
>  		     unsigned long *size)
>  {
> -	void *data = read_object(sha1, type, size);
> +	const unsigned char *repl = lookup_replace_object(sha1);
> +	void *data = read_object(repl, type, size);
> +
> +	/* die if we replaced an object with one that does not exist */
> +	if (!data && repl != sha1)
> +		die("replacement %s not found for %s",
> +		    sha1_to_hex(repl), sha1_to_hex(sha1));
> +
>  	/* legacy behavior is to die on corrupted objects */
> -	if (!data && (has_loose_object(sha1) || has_packed_and_bad(sha1)))
> -		die("object %s is corrupted", sha1_to_hex(sha1));
> +	if (!data && (has_loose_object(repl) || has_packed_and_bad(repl)))
> +		die("object %s is corrupted", sha1_to_hex(repl));
> +
>  	return data;
>  }

Later we'd need a global switch to forbid the replacement for connectivity
walkers, but other than that I think this is sane.

I also looked at 1/ and 2/ which looked Ok.

Thanks.

^ permalink raw reply

* [PATCH,v2] contrib/examples/README: give an explanation of the status of these files
From: jidanni @ 2009-01-13  1:19 UTC (permalink / raw)
  To: gitster; +Cc: git
In-Reply-To: <7viqokf21j.fsf@gitster.siamese.dyndns.org>

We attempt to give an explanation of the status of the files in this
directory.

Signed-off-by: jidanni <jidanni@jidanni.org>
---
 contrib/examples/README |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
 create mode 100644 contrib/examples/README

diff --git a/contrib/examples/README b/contrib/examples/README
new file mode 100644
index 0000000..6946f3d
--- /dev/null
+++ b/contrib/examples/README
@@ -0,0 +1,3 @@
+These are original scripted implementations, kept primarily for their
+reference value to any aspiring plumbing users who want to learn how
+pieces can be fit together.
-- 
1.6.0.6

^ permalink raw reply related

* [PATCH/RFC] Documentation/git-blame.txt, git-gui.txt: link SEE ALSOs
From: jidanni @ 2009-01-13  1:13 UTC (permalink / raw)
  To: git

As git gui is heavily blame focused, we link its SEE ALSO to
git-blame, and add a link back while we're at it.

Signed-off-by: jidanni <jidanni@jidanni.org>
---
 Documentation/git-blame.txt |    1 +
 Documentation/git-gui.txt   |    3 +++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index fba374d..d71a2c3 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -186,6 +186,7 @@ commit commentary), a blame viewer won't ever care.
 
 SEE ALSO
 --------
+linkgit:git-gui[1],
 linkgit:git-annotate[1]
 
 AUTHOR
diff --git a/Documentation/git-gui.txt b/Documentation/git-gui.txt
index d0bc98b..3a71074 100644
--- a/Documentation/git-gui.txt
+++ b/Documentation/git-gui.txt
@@ -105,6 +105,9 @@ linkgit:gitk[1]::
 	and file differences.  gitk is the utility started by
 	'git-gui''s Repository Visualize actions.
 
+linkgit:git-blame[1]::
+	Command-line blame viewer.
+
 Other
 -----
 'git-gui' is actually maintained as an independent project, but stable
-- 
1.6.0.6

^ permalink raw reply related

* Re: [PATCH/RFC] contrib/examples/README: warn of obsolescence
From: Junio C Hamano @ 2009-01-13  1:05 UTC (permalink / raw)
  To: jidanni; +Cc: git
In-Reply-To: <87fxjoowyp.fsf@jidanni.org>

jidanni@jidanni.org writes:

> We attempt to give an explanation of the status of the files in this
> directory.

Yeah, that is a good discipline.

They are original scripted implementations, kept primarily for their
reference value to any aspiring plumbing users who want to learn how
pieces can be fit together.

^ permalink raw reply

* Re: [PATCH] Teach format-patch to handle output directory relative to cwd
From: Junio C Hamano @ 2009-01-13  1:00 UTC (permalink / raw)
  To: Cesar Eduardo Barros; +Cc: git
In-Reply-To: <496BCE55.8030407@cesarb.net>

Cesar Eduardo Barros <cesarb@cesarb.net> writes:

> Works great, only the resulting output to the screen is a bit
> ugly/confusing:
>
> drivers/net/../../../0001-sc92031-more-useful-banner-in-kernel-log.patch
> ...
> (after all, I am still inside the drivers/net directory)

Fair enough.  Here is a replacement diff.

 builtin-log.c           |   28 ++++++++++++++++++++++--
 t/t4014-format-patch.sh |   52 ++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 76 insertions(+), 4 deletions(-)

diff --git c/builtin-log.c w/builtin-log.c
index db71e0d..16a0f11 100644
--- c/builtin-log.c
+++ w/builtin-log.c
@@ -568,6 +568,7 @@ static const char *get_oneline_for_filename(struct commit *commit,
 
 static FILE *realstdout = NULL;
 static const char *output_directory = NULL;
+static int outdir_offset;
 
 static int reopen_stdout(const char *oneline, int nr, int total)
 {
@@ -594,7 +595,7 @@ static int reopen_stdout(const char *oneline, int nr, int total)
 		strcpy(filename + len, fmt_patch_suffix);
 	}
 
-	fprintf(realstdout, "%s\n", filename);
+	fprintf(realstdout, "%s\n", filename + outdir_offset);
 	if (freopen(filename, "w", stdout) == NULL)
 		return error("Cannot open patch file %s",filename);
 
@@ -757,6 +758,27 @@ static const char *clean_message_id(const char *msg_id)
 	return xmemdupz(a, z - a);
 }
 
+static const char *set_outdir(const char *prefix, const char *output_directory)
+{
+	if (output_directory && is_absolute_path(output_directory))
+		return output_directory;
+
+	if (!prefix || !*prefix) {
+		if (output_directory)
+			return output_directory;
+		/* The user did not explicitly ask for "./" */
+		outdir_offset = 2;
+		return "./";
+	}
+
+	outdir_offset = strlen(prefix);
+	if (!output_directory)
+		return prefix;
+
+	return xstrdup(prefix_filename(prefix, outdir_offset,
+				       output_directory));
+}
+
 int cmd_format_patch(int argc, const char **argv, const char *prefix)
 {
 	struct commit *commit;
@@ -935,8 +957,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	if (!DIFF_OPT_TST(&rev.diffopt, TEXT) && !no_binary_diff)
 		DIFF_OPT_SET(&rev.diffopt, BINARY);
 
-	if (!output_directory && !use_stdout)
-		output_directory = prefix;
+	if (!use_stdout)
+		output_directory = set_outdir(prefix, output_directory);
 
 	if (output_directory) {
 		if (use_stdout)
diff --git c/t/t4014-format-patch.sh w/t/t4014-format-patch.sh
index 7fe853c..16de436 100755
--- c/t/t4014-format-patch.sh
+++ w/t/t4014-format-patch.sh
@@ -3,7 +3,7 @@
 # Copyright (c) 2006 Junio C Hamano
 #
 
-test_description='Format-patch skipping already incorporated patches'
+test_description='various format-patch tests'
 
 . ./test-lib.sh
 
@@ -230,4 +230,54 @@ test_expect_success 'shortlog of cover-letter wraps overly-long onelines' '
 
 '
 
+test_expect_success 'format-patch from a subdirectory (1)' '
+	filename=$(
+		rm -rf sub &&
+		mkdir -p sub/dir &&
+		cd sub/dir &&
+		git format-patch -1
+	) &&
+	case "$filename" in
+	0*)
+		;; # ok
+	*)
+		echo "Oops? $filename"
+		false
+		;;
+	esac &&
+	test -f "$filename"
+'
+
+test_expect_success 'format-patch from a subdirectory (2)' '
+	filename=$(
+		rm -rf sub &&
+		mkdir -p sub/dir &&
+		cd sub/dir &&
+		git format-patch -1 -o ..
+	) &&
+	case "$filename" in
+	../0*)
+		;; # ok
+	*)
+		echo "Oops? $filename"
+		false
+		;;
+	esac &&
+	basename=$(expr "$filename" : ".*/\(.*\)") &&
+	test -f "sub/$basename"
+'
+
+test_expect_success 'format-patch from a subdirectory (3)' '
+	here="$TEST_DIRECTORY/$test" &&
+	rm -f 0* &&
+	filename=$(
+		rm -rf sub &&
+		mkdir -p sub/dir &&
+		cd sub/dir &&
+		git format-patch -1 -o "$here"
+	) &&
+	basename=$(expr "$filename" : ".*/\(.*\)") &&
+	test -f "$basename"
+'
+
 test_done

^ permalink raw reply related

* Re: Help! My ISP blocks repo.or.cz. How to push changes?
From: Markus Heidelberg @ 2009-01-13  0:59 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Mike Hommey, Alex Riesen, git
In-Reply-To: <200901130043.04772.jnareb@gmail.com>

Jakub Narebski, 13.01.2009:
> P.S. What should I put in core.gitProxy to make it possible to fetch
> via git:// protocol from repo.or.cz?

I'm not sure if this is what you need, but I use this at work for
fetching via git protocol:

[core]
	gitProxy = /etc/gitproxy.sh for kernel.org
	gitProxy = /etc/gitproxy.sh for or.cz
	# and several others ...

gitproxy.sh:
#! /bin/sh
(echo "CONNECT $1:$2 HTTP/1.0"; echo; cat ) | socket <company proxy host> <port> | (read a; read a; cat )

Markus

^ permalink raw reply

* Re: [BUG/RFH] gitweb: Trouble with ref markers being hyperlinks because of illegally nested links
From: Giuseppe Bilotta @ 2009-01-13  0:59 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <200901130113.54517.jnareb@gmail.com>

On Mon, Jan 12, 2009 at 7:13 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> On Mon, 12 Jan 2009, Giuseppe Bilotta wrote:
>> On Sun, Jan 11, 2009 at 8:15 PM, Jakub Narebski <jnareb@gmail.com> wrote:
>
>> > I see possible the following alternate solutions:
>> >  * Ignore this issue (e.g. if it does not affect modern browsers)
>>
>> That would be my current choice until we find a better solution.
>
> By the way, how common this error is? Could you check if _your_ web
> browser (Firefox, Internet Explorer, Opera, Konqueror, Safari, Chrome)
> does show this bug or not, please?

Opera works fine (no display or functionality anomaly). That makes
sense, since I was the one who submitted the patch 8-D. Konqueror
3.5.9 does the ugly thing instead.

Notice that nested links are actually valid *XML*. Indeed, I asked on
www-style and they suggested leaving the problem as-is, serving as
html+xml which is what we do.

>> >  * Revert 4afbaef (we lose feature, but how often used is it?)
>> >  * Always use quirks mode, or check browser and use quirks mode if it
>> >    would break layout
>> >  * Use extra divs and links and CSS positioning to make layout which
>> >    looks like current one, and behaves as current one, but is more
>> >    complicated.
>>
>> I'm asking on #html, hopefully I'll get some interesting idea to try for this.
>
> I have found _a_ solution. Perhaps not the best one, but it works.
> And IMHO gives / can give even better visual.
>
> Current version (line wrapped for better visibility):
>  <div class="header">
>  <a class="title" href="...">GIT 1.6.1
>  <span class="refs">
>    <span class="tag indirect" title="tags/v1.6.1">
>    <a href="...">v1.6.1</a>
>    </span>
>  </span>
>  </a>
>  </div>
>
> Current CSS (relevant part):
>  a.title {
>        display: block;
>        padding: 6px 8px;
>  }
>
> Current rendering:
>  -----------------------------------------------------------
>  |_GIT 1.6.1_ []                                           |
>  -----------------------------------------------------------
>  __v1.6.1__
>
>
> Proposed code (line wrapped for better visibility, with CSS embedded,
> which would change in final version of course). Only parts of style
> related to positioning are shown.
>  <div class="header">
>  <a href="..." style="float: left; margin: 6px 1px 6px 8px;">GIT 1.6.1</a>
>  <div style="float: left; margin: 6px 1px;">
>    <span class="refs">
>      <span class="tag indirect" title="tags/v1.6.1">
>      <a href="...">v1.6.1</a>
>      </span>
>    </span>
>  </div>
>  <a href="..." style="display: block; padding: 6px 8px;">&nbsp;</a>
>  </div>
>
> Rendering with proposed code:
>  -----------------------------------------------------------
>  _|_GIT 1.6.1_ [_v1.6.1_]                                 |_
>  -----------------------------------------------------------
>
> I guess that instead of additional DIV element, we could use DIV for
> .refs, or use "float: right" style with SPAN element. I have not
> checked if other variations works: this one does.
>
> What do you think?

The float thing was the second suggestion I was given on www-style.
Can you provide a patch I can apply to my tree for testing to see how
it comes up?


-- 
Giuseppe "Oblomov" Bilotta

^ permalink raw reply

* Re: git/webdav is refusing to authenticate properly.
From: Peter Spierenburg @ 2009-01-13  0:35 UTC (permalink / raw)
  Cc: git
In-Reply-To: <alpine.DEB.1.00.0901130003490.3586@pacific.mpi-cbg.de>

C'mon, leave my password 'in-the-clear', in a text file on a networked 
box? That is the kind of prank a fourth-year student would try to pull 
on a freshman.

How do I really do it?

Peter.


Johannes Schindelin wrote:
> Hi,
>
> On Mon, 12 Jan 2009, Peter Spierenburg wrote:
>
>   
>> I'm trying to push a local git repository to a remote site using WebDAV, 
>> but it is eating my lunch.
>>     
>
> Please see Documentation/howto/setup-git-server-over-http.txt.
>
> In short, http-push does not ask for a password interactively, but you 
> have to use .netrc.
>
> Hth,
> Dscho
>
>   

^ permalink raw reply

* Re: [PATCH v2] make diff --color-words customizable
From: Jakub Narebski @ 2009-01-13  0:52 UTC (permalink / raw)
  To: Davide Libenzi; +Cc: Johannes Schindelin, git, Thomas Rast
In-Reply-To: <alpine.DEB.1.10.0901100950230.21891@alien.or.mcafeemobile.com>

On Sat, 10 Jan 2009, Davide Libenzi wrote:
> On Sat, 10 Jan 2009, Jakub Narebski wrote:
>> On Sat, 10 Jan 2009, Johannes Schindelin wrote:
>>> On Sat, 10 Jan 2009, Jakub Narebski wrote:
>>>> Thomas Rast wrote:
>>>> 
>>>>> --color-words works (and always worked) by splitting words onto one
>>>>> line each, and using the normal line-diff machinery to get a word
>>>>> diff. 
>>>> 
>>>> Cannot we generalize diff machinery / use underlying LCS diff engine
>>>> instead of going through line diff?
>>> 
>>> What do you think we're doing?  libxdiff is pretty hardcoded to newlines.  
>>> That's why we're substituting non-word characters with newlines.
>> 
>> Isn't Meyers algorithm used by libxdiff based on LCS, largest common
>> subsequence, and doesn't it generate from the mathematical point of
>> view "diff" between two sequences (two arrays) which just happen to
>> be lines? It is a bit strange that libxdiff doesn't export its low
>> level algorithm...
> 
> The core doesn't know anything about lines. Only pre-processing (setting 
> up the hash by tokenizing the input) and post-processing (adding '\n' to 
> the end of each token), knows about newlines. Memory consumption would 
> increase significantly though, since there is a per-token cost, and a 
> word-based diff will create more of them WRT the same input.

Is this core algorithm available as some exported function in libxdiff?
I mean would it be easy to replace default line tokenizer (per-line
pre-processing) and post-processing to better deal with word diff?

The other side would be to generate per-paragraph diffs (with empty
line being separator)...
-- 
Jakub Narebski
Poland

^ permalink raw reply

* [PATCH/RFC] contrib/examples/README: warn of obsolescence
From: jidanni @ 2009-01-13  0:45 UTC (permalink / raw)
  To: git

We attempt to give an explanation of the status of the files in this
directory.

Signed-off-by: jidanni <jidanni@jidanni.org>
---
 contrib/examples/README |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
 create mode 100644 contrib/examples/README

diff --git a/contrib/examples/README b/contrib/examples/README
new file mode 100644
index 0000000..b10910c
--- /dev/null
+++ b/contrib/examples/README
@@ -0,0 +1 @@
+Note that these are obsolete versions of programs now replaced by C code.
-- 
1.6.0.6

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox