git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH v3 0/4] git gui: allow for a long recentrepo list
@ 2017-01-22 19:52 Philip Oakley
  2017-01-22 19:52 ` [PATCH v3 1/4] git-gui: remove duplicate entries from .gitconfig's gui.recentrepo Philip Oakley
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Philip Oakley @ 2017-01-22 19:52 UTC (permalink / raw)
  To: GitList
  Cc: Self, Junio C Hamano, Pat Thoyts, Eric Sunshine, Alexey Astakhov,
	Johannes Schindelin

Way back in December 2015 I made a couple of attempts to patch up
the git-gui's recentrepo list in the face of duplicate entries in
the .gitconfig

The series end at http://public-inbox.org/git/9731888BD4C348F5BFC82FA96D978034@PhilipOakley/

A similar problem was reported recently on the Git for Windows list
https://github.com/git-for-windows/git/issues/1014 were a full
recentrepo list also stopped the gui working.

This series applies to Pat Thoyt's upstream Github repo as a merge
ready Pull Request https://github.com/patthoyts/git-gui/pull/10.
Hence if applied here it should be into the sub-tree git/git-gui.

I believe I've covered the points raised by Junio in the last review
and allowed for cases where the recentrepo list is now longer than the
maximum (which can be configured, both up and down).

I've cc'd just the cover letter to those involved previously, rather
than the series to avoid spamming.

There are various other low hanging fruit that the eager could look at
which are detailed at the end of the GfW/issues/104 referenced above.

Philip Oakley (4):
  git-gui: remove duplicate entries from .gitconfig's gui.recentrepo
  git gui: cope with duplicates in _get_recentrepo
  git gui: de-dup selected repo from recentrepo history
  git gui: allow for a long recentrepo list

 lib/choose_repository.tcl | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

-- 
2.9.0.windows.1.323.g0305acf
---
cc: Junio C Hamano <gitster@pobox.com>
cc: Pat Thoyts <patthoyts@users.sourceforge.net>,
cc: Eric Sunshine <sunshine@sunshineco.com>,
cc: Alexey Astakhov <asstv7@gmail.com>
cc: Johannes Schindelin <johannes.schindelin@gmx.de>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 1/4] git-gui: remove duplicate entries from .gitconfig's gui.recentrepo
  2017-01-22 19:52 [PATCH v3 0/4] git gui: allow for a long recentrepo list Philip Oakley
@ 2017-01-22 19:52 ` Philip Oakley
  2017-01-22 19:52 ` [PATCH v3 2/4] git gui: cope with duplicates in _get_recentrepo Philip Oakley
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Philip Oakley @ 2017-01-22 19:52 UTC (permalink / raw)
  To: GitList; +Cc: Self

The git gui's recent repo list may become contaminated with duplicate
entries. The git gui would barf when attempting to remove one entry.
Remove them all - there is no option within 'git config' to selectively
remove one of the entries.

This issue was reported on the 'Git User' list
(https://groups.google.com/forum/#!topic/git-users/msev4KsQGFc,
Warning: gui.recentrepo has multiply values while executing).

And also by zosrothko as a Git-for-Windows issue
https://github.com/git-for-windows/git/issues/1014.

On startup the gui checks that entries in the recentrepo list are still
valid repos and deletes thoses that are not. If duplicate entries are
present the 'git config --unset' will barf and this prevents the gui
from starting.

Subsequent patches fix other parts of recentrepo logic used for syncing
internal lists with the external .gitconfig.

Reported-by: Alexey Astakhov <asstv7@gmail.com>
Signed-off-by: Philip Oakley <philipoakley@iee.org>
---
 lib/choose_repository.tcl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl
index 75d1da8..133ca0a 100644
--- a/lib/choose_repository.tcl
+++ b/lib/choose_repository.tcl
@@ -247,7 +247,7 @@ proc _get_recentrepos {} {
 
 proc _unset_recentrepo {p} {
 	regsub -all -- {([()\[\]{}\.^$+*?\\])} $p {\\\1} p
-	git config --global --unset gui.recentrepo "^$p\$"
+	git config --global --unset-all gui.recentrepo "^$p\$"
 	load_config 1
 }
 
-- 
2.9.0.windows.1.323.g0305acf


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 2/4] git gui: cope with duplicates in _get_recentrepo
  2017-01-22 19:52 [PATCH v3 0/4] git gui: allow for a long recentrepo list Philip Oakley
  2017-01-22 19:52 ` [PATCH v3 1/4] git-gui: remove duplicate entries from .gitconfig's gui.recentrepo Philip Oakley
@ 2017-01-22 19:52 ` Philip Oakley
  2017-01-22 19:53 ` [PATCH v3 3/4] git gui: de-dup selected repo from recentrepo history Philip Oakley
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Philip Oakley @ 2017-01-22 19:52 UTC (permalink / raw)
  To: GitList; +Cc: Self

_get_recentrepo will fail if duplicate invalid entries are present
in the recentrepo config list. The previous commit fixed the
'git config' limitations in _unset_recentrepo by unsetting all config
entries, however this code would fail on the second attempt to unset it.

Refactor the code to pre-sort and de-duplicate the recentrepo list to
avoid a potential second unset attempt.

Signed-off-by: Philip Oakley <philipoakley@iee.org>
---
 lib/choose_repository.tcl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl
index 133ca0a..aa87bcc 100644
--- a/lib/choose_repository.tcl
+++ b/lib/choose_repository.tcl
@@ -235,14 +235,14 @@ method _invoke_next {} {
 
 proc _get_recentrepos {} {
 	set recent [list]
-	foreach p [get_config gui.recentrepo] {
+	foreach p [lsort -unique [get_config gui.recentrepo]] {
 		if {[_is_git [file join $p .git]]} {
 			lappend recent $p
 		} else {
 			_unset_recentrepo $p
 		}
 	}
-	return [lsort $recent]
+	return $recent
 }
 
 proc _unset_recentrepo {p} {
-- 
2.9.0.windows.1.323.g0305acf


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 3/4] git gui: de-dup selected repo from recentrepo history
  2017-01-22 19:52 [PATCH v3 0/4] git gui: allow for a long recentrepo list Philip Oakley
  2017-01-22 19:52 ` [PATCH v3 1/4] git-gui: remove duplicate entries from .gitconfig's gui.recentrepo Philip Oakley
  2017-01-22 19:52 ` [PATCH v3 2/4] git gui: cope with duplicates in _get_recentrepo Philip Oakley
@ 2017-01-22 19:53 ` Philip Oakley
  2017-01-22 19:53 ` [PATCH v3 4/4] git gui: allow for a long recentrepo list Philip Oakley
  2017-02-11 12:48 ` [PATCH v3 0/4] " Philip Oakley
  4 siblings, 0 replies; 6+ messages in thread
From: Philip Oakley @ 2017-01-22 19:53 UTC (permalink / raw)
  To: GitList; +Cc: Self

When the gui/user selects a repo for display, that repo is brought to
the end of the recentrepo config list. The logic can fail if there are
duplicate old entries for the repo (you cannot unset a single config
entry when duplicates are present).

Similarly, the maxrecentrepo logic could fail if older duplicate entries
are present.

The first commit of this series ({this}~2) fixed the config unsetting
issue. Rather than manipulating a local copy of the $recent list (one
cannot know how many entries were removed), simply re-read it.

We must also catch the error when the attempt to remove the second copy
from the re-read list is performed.

Signed-off-by: Philip Oakley <philipoakley@iee.org>
---
 lib/choose_repository.tcl | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl
index aa87bcc..f39636f 100644
--- a/lib/choose_repository.tcl
+++ b/lib/choose_repository.tcl
@@ -247,7 +247,7 @@ proc _get_recentrepos {} {
 
 proc _unset_recentrepo {p} {
 	regsub -all -- {([()\[\]{}\.^$+*?\\])} $p {\\\1} p
-	git config --global --unset-all gui.recentrepo "^$p\$"
+	catch {git config --global --unset-all gui.recentrepo "^$p\$"}
 	load_config 1
 }
 
@@ -262,12 +262,11 @@ proc _append_recentrepos {path} {
 	set i [lsearch $recent $path]
 	if {$i >= 0} {
 		_unset_recentrepo $path
-		set recent [lreplace $recent $i $i]
 	}
 
-	lappend recent $path
 	git config --global --add gui.recentrepo $path
 	load_config 1
+	set recent [get_config gui.recentrepo]
 
 	if {[set maxrecent [get_config gui.maxrecentrepo]] eq {}} {
 		set maxrecent 10
@@ -275,7 +274,7 @@ proc _append_recentrepos {path} {
 
 	while {[llength $recent] > $maxrecent} {
 		_unset_recentrepo [lindex $recent 0]
-		set recent [lrange $recent 1 end]
+		set recent [get_config gui.recentrepo]
 	}
 }
 
-- 
2.9.0.windows.1.323.g0305acf


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 4/4] git gui: allow for a long recentrepo list
  2017-01-22 19:52 [PATCH v3 0/4] git gui: allow for a long recentrepo list Philip Oakley
                   ` (2 preceding siblings ...)
  2017-01-22 19:53 ` [PATCH v3 3/4] git gui: de-dup selected repo from recentrepo history Philip Oakley
@ 2017-01-22 19:53 ` Philip Oakley
  2017-02-11 12:48 ` [PATCH v3 0/4] " Philip Oakley
  4 siblings, 0 replies; 6+ messages in thread
From: Philip Oakley @ 2017-01-22 19:53 UTC (permalink / raw)
  To: GitList; +Cc: Self

The gui.recentrepo list may be longer than the maxrecent setting.
Allow extra space to show any extra entries.

In an ideal world, the git gui would limit the number of entries
to the maxrecent setting, however the recentrepo config list may
have been extended outwith the gui, or the maxrecent setting changed
to a reduced value. Further, when testing the gui's recentrepo
logic it is useful to show these extra, but valid, entries.

Signed-off-by: Philip Oakley <philipoakley@iee.org>
---
 lib/choose_repository.tcl | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl
index f39636f..80f5a59 100644
--- a/lib/choose_repository.tcl
+++ b/lib/choose_repository.tcl
@@ -142,6 +142,10 @@ constructor pick {} {
 				-label [mc "Recent Repositories"]
 		}
 
+	if {[set lenrecent [llength $sorted_recent]] < $maxrecent} {
+		set lenrecent $maxrecent
+	}
+
 		${NS}::label $w_body.space
 		${NS}::label $w_body.recentlabel \
 			-anchor w \
@@ -153,7 +157,7 @@ constructor pick {} {
 			-background [get_bg_color $w_body.recentlabel] \
 			-wrap none \
 			-width 50 \
-			-height $maxrecent
+			-height $lenrecent
 		$w_recentlist tag conf link \
 			-foreground blue \
 			-underline 1
-- 
2.9.0.windows.1.323.g0305acf


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 0/4] git gui: allow for a long recentrepo list
  2017-01-22 19:52 [PATCH v3 0/4] git gui: allow for a long recentrepo list Philip Oakley
                   ` (3 preceding siblings ...)
  2017-01-22 19:53 ` [PATCH v3 4/4] git gui: allow for a long recentrepo list Philip Oakley
@ 2017-02-11 12:48 ` Philip Oakley
  4 siblings, 0 replies; 6+ messages in thread
From: Philip Oakley @ 2017-02-11 12:48 UTC (permalink / raw)
  To: GitList
  Cc: Junio C Hamano, Pat Thoyts, Eric Sunshine, Alexey Astakhov,
	Johannes Schindelin

Ping

Any comments anyone?
(https://public-inbox.org/git/20170122195301.1784-1-philipoakley@iee.org/)

I understand that the Git-for-Windows team is planning to include this in 
their next release, so additional eyes are welcomed.

Philip

From: "Philip Oakley" <philipoakley@iee.org> date: Sunday, January 22, 2017 
7:52 PM
> Way back in December 2015 I made a couple of attempts to patch up
> the git-gui's recentrepo list in the face of duplicate entries in
> the .gitconfig
>
> The series end at 
> http://public-inbox.org/git/9731888BD4C348F5BFC82FA96D978034@PhilipOakley/
>
> A similar problem was reported recently on the Git for Windows list
> https://github.com/git-for-windows/git/issues/1014 were a full
> recentrepo list also stopped the gui working.
>
> This series applies to Pat Thoyt's upstream Github repo as a merge
> ready Pull Request https://github.com/patthoyts/git-gui/pull/10.
> Hence if applied here it should be into the sub-tree git/git-gui.
>
> I believe I've covered the points raised by Junio in the last review
> and allowed for cases where the recentrepo list is now longer than the
> maximum (which can be configured, both up and down).
>
> I've cc'd just the cover letter to those involved previously, rather
> than the series to avoid spamming.
>
> There are various other low hanging fruit that the eager could look at
> which are detailed at the end of the GfW/issues/104 referenced above.
>
> Philip Oakley (4):
>  git-gui: remove duplicate entries from .gitconfig's gui.recentrepo
>  git gui: cope with duplicates in _get_recentrepo
>  git gui: de-dup selected repo from recentrepo history
>  git gui: allow for a long recentrepo list
>
> lib/choose_repository.tcl | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> -- 
> 2.9.0.windows.1.323.g0305acf
> ---
> cc: Junio C Hamano <gitster@pobox.com>
> cc: Pat Thoyts <patthoyts@users.sourceforge.net>,
> cc: Eric Sunshine <sunshine@sunshineco.com>,
> cc: Alexey Astakhov <asstv7@gmail.com>
> cc: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-02-11 12:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-22 19:52 [PATCH v3 0/4] git gui: allow for a long recentrepo list Philip Oakley
2017-01-22 19:52 ` [PATCH v3 1/4] git-gui: remove duplicate entries from .gitconfig's gui.recentrepo Philip Oakley
2017-01-22 19:52 ` [PATCH v3 2/4] git gui: cope with duplicates in _get_recentrepo Philip Oakley
2017-01-22 19:53 ` [PATCH v3 3/4] git gui: de-dup selected repo from recentrepo history Philip Oakley
2017-01-22 19:53 ` [PATCH v3 4/4] git gui: allow for a long recentrepo list Philip Oakley
2017-02-11 12:48 ` [PATCH v3 0/4] " Philip Oakley

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).