git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* style(git-gui): Fix mixed tabs & spaces; Always use tabs.
@ 2020-08-22 10:56 Serg Tereshchenko
  2020-08-22 19:26 ` Junio C Hamano
  0 siblings, 1 reply; 6+ messages in thread
From: Serg Tereshchenko @ 2020-08-22 10:56 UTC (permalink / raw)
  To: git; +Cc: Serg Tereshchenko

Hello.

I want to improve styling of git-citool, so it supports dark themes.
But first i want to remove "mixed indent warning" from my editor
modeline.

This patch does not change anything besides indents, and sometimes i
replace

```tcl
if {long ||
    multiline ||
    statement}
```

with more readable

```tcl
if {
    long
    || multiline
    || statement
}
```
But only in lines with mixed indents.


To clearly see changes in this patch, instruct your editor to display
tabs.
I use this in vim:

```vim
set list
set listchars=eol:¬,tab:\ \ ┊,trail:·,nbsp:⎵
```


Btw, at the start of git-gui.sh there is sanity check requiring Tcl 8.5.
And somewhere later legacy code for 8.4.

I would like to clean it up. Should i do this or better leave it as it
is?


diff --git a/git-gui.sh b/git-gui.sh
index 49bd86e..ca66a8e 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1,13 +1,10 @@
 #!/bin/sh
 # Tcl ignores the next line -*- tcl -*- \
- if test "z$*" = zversion \
- || test "z$*" = z--version; \
- then \
-	echo 'git-gui version @@GITGUI_VERSION@@'; \
-	exit; \
- fi; \
- argv0=$0; \
- exec wish "$argv0" -- "$@"
+if test "z$*" = zversion || test "z$*" = z--version; \
+	then echo 'git-gui version @@GITGUI_VERSION@@'; exit; \
+fi; \
+argv0=$0; \
+exec wish "$argv0" -- "$@"
 
 set appvers {@@GITGUI_VERSION@@}
 set copyright [string map [list (c) \u00a9] {
@@ -31,7 +28,7 @@ along with this program; if not, see <http://www.gnu.org/licenses/>.}]
 ## Tcl/Tk sanity check
 
 if {[catch {package require Tcl 8.5} err]
- || [catch {package require Tk  8.5} err]
+	|| [catch {package require Tk  8.5} err]
 } {
 	catch {wm withdraw .}
 	tk_messageBox \
@@ -404,9 +401,9 @@ proc _git_cmd {name} {
 
 	if {[catch {set v $_git_cmd_path($name)}]} {
 		switch -- $name {
-		  version   -
-		--version   -
-		--exec-path { return [list $::_git $name] }
+			version     -
+			--version   -
+			--exec-path { return [list $::_git $name] }
 		}
 
 		set p [gitexec git-$name$::_search_exe]
@@ -548,9 +545,10 @@ proc _open_stdout_stderr {cmd} {
 	if {[catch {
 			set fd [open [concat [list | ] $cmd] r]
 		} err]} {
-		if {   [lindex $cmd end] eq {2>@1}
-		    && $err eq {can not find channel named "1"}
-			} {
+		if {
+			[lindex $cmd end] eq {2>@1}
+			&& $err eq {can not find channel named "1"}
+		} {
 			# Older versions of Tcl 8.4 don't have this 2>@1 IO
 			# redirect operator.  Fallback to |& cat for those.
 			# The command was not actually started, so its safe
@@ -947,15 +945,15 @@ if {![regsub {^git version } $_git_version {} _git_version]} {
 }
 
 proc get_trimmed_version {s} {
-    set r {}
-    foreach x [split $s -._] {
-        if {[string is integer -strict $x]} {
-            lappend r $x
-        } else {
-            break
-        }
-    }
-    return [join $r .]
+	set r {}
+	foreach x [split $s -._] {
+		if {[string is integer -strict $x]} {
+			lappend r $x
+		} else {
+			break
+		}
+	}
+	return [join $r .]
 }
 set _real_git_version $_git_version
 set _git_version [get_trimmed_version $_git_version]
@@ -967,7 +965,7 @@ if {![regexp {^[1-9]+(\.[0-9]+)+$} $_git_version]} {
 		-type yesno \
 		-default no \
 		-title "[appname]: warning" \
-		 -message [mc "Git version cannot be determined.
+		-message [mc "Git version cannot be determined.
 
 %s claims it is version '%s'.
 
@@ -975,7 +973,7 @@ if {![regexp {^[1-9]+(\.[0-9]+)+$} $_git_version]} {
 
 Assume '%s' is version 1.5.0?
 " $_git $_real_git_version [appname] $_real_git_version]] eq {yes}} {
-		set _git_version 1.5.0
+	set _git_version 1.5.0
 	} else {
 		exit 1
 	}
@@ -1181,44 +1179,44 @@ enable_option transport
 disable_option bare
 
 switch -- $subcommand {
-browser -
-blame {
-	enable_option bare
-
-	disable_option multicommit
-	disable_option branch
-	disable_option transport
-}
-citool {
-	enable_option singlecommit
-	enable_option retcode
-
-	disable_option multicommit
-	disable_option branch
-	disable_option transport
+	browser -
+	blame {
+		enable_option bare
+
+		disable_option multicommit
+		disable_option branch
+		disable_option transport
+	}
+	citool {
+		enable_option singlecommit
+		enable_option retcode
+
+		disable_option multicommit
+		disable_option branch
+		disable_option transport
+
+		while {[llength $argv] > 0} {
+			set a [lindex $argv 0]
+			switch -- $a {
+				--amend {
+					enable_option initialamend
+				}
+				--nocommit {
+					enable_option nocommit
+					enable_option nocommitmsg
+				}
+				--commitmsg {
+					disable_option nocommitmsg
+				}
+				default {
+					break
+				}
+			}
 
-	while {[llength $argv] > 0} {
-		set a [lindex $argv 0]
-		switch -- $a {
-		--amend {
-			enable_option initialamend
+			set argv [lrange $argv 1 end]
 		}
-		--nocommit {
-			enable_option nocommit
-			enable_option nocommitmsg
-		}
-		--commitmsg {
-			disable_option nocommitmsg
-		}
-		default {
-			break
-		}
-		}
-
-		set argv [lrange $argv 1 end]
 	}
 }
-}
 
 ######################################################################
 ##
@@ -1237,15 +1235,15 @@ if {![info exists env(SSH_ASKPASS)]} {
 
 set picked 0
 if {[catch {
-		set _gitdir $env(GIT_DIR)
-		set _prefix {}
-		}]
-	&& [catch {
-		# beware that from the .git dir this sets _gitdir to .
-		# and _prefix to the empty string
-		set _gitdir [git rev-parse --git-dir]
-		set _prefix [git rev-parse --show-prefix]
-	} err]} {
+	set _gitdir $env(GIT_DIR)
+	set _prefix {}
+}]
+&& [catch {
+	# beware that from the .git dir this sets _gitdir to .
+	# and _prefix to the empty string
+	set _gitdir [git rev-parse --git-dir]
+	set _prefix [git rev-parse --show-prefix]
+} err]} {
 	load_config 1
 	apply_config
 	choose_repository::pick
@@ -1653,7 +1651,7 @@ proc prepare_commit_msg_hook_wait {fd_ph} {
 		set pch_error {}
 		catch {file delete [gitdir PREPARE_COMMIT_MSG]}
 		return
-        }
+	}
 	fconfigure $fd_ph -blocking 0
 	catch {file delete [gitdir PREPARE_COMMIT_MSG]}
 }
@@ -2001,72 +1999,72 @@ set filemask {
 #define mask_width 14
 #define mask_height 15
 static unsigned char mask_bits[] = {
-   0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f,
-   0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f,
-   0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f};
+	0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f,
+	0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f,
+	0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f};
 }
 
 image create bitmap file_plain -background white -foreground black -data {
 #define plain_width 14
 #define plain_height 15
 static unsigned char plain_bits[] = {
-   0xfe, 0x01, 0x02, 0x03, 0x02, 0x05, 0x02, 0x09, 0x02, 0x1f, 0x02, 0x10,
-   0x02, 0x10, 0x02, 0x10, 0x02, 0x10, 0x02, 0x10, 0x02, 0x10, 0x02, 0x10,
-   0x02, 0x10, 0x02, 0x10, 0xfe, 0x1f};
+	0xfe, 0x01, 0x02, 0x03, 0x02, 0x05, 0x02, 0x09, 0x02, 0x1f, 0x02, 0x10,
+	0x02, 0x10, 0x02, 0x10, 0x02, 0x10, 0x02, 0x10, 0x02, 0x10, 0x02, 0x10,
+	0x02, 0x10, 0x02, 0x10, 0xfe, 0x1f};
 } -maskdata $filemask
 
 image create bitmap file_mod -background white -foreground blue -data {
 #define mod_width 14
 #define mod_height 15
 static unsigned char mod_bits[] = {
-   0xfe, 0x01, 0x02, 0x03, 0x7a, 0x05, 0x02, 0x09, 0x7a, 0x1f, 0x02, 0x10,
-   0xfa, 0x17, 0x02, 0x10, 0xfa, 0x17, 0x02, 0x10, 0xfa, 0x17, 0x02, 0x10,
-   0xfa, 0x17, 0x02, 0x10, 0xfe, 0x1f};
+	0xfe, 0x01, 0x02, 0x03, 0x7a, 0x05, 0x02, 0x09, 0x7a, 0x1f, 0x02, 0x10,
+	0xfa, 0x17, 0x02, 0x10, 0xfa, 0x17, 0x02, 0x10, 0xfa, 0x17, 0x02, 0x10,
+	0xfa, 0x17, 0x02, 0x10, 0xfe, 0x1f};
 } -maskdata $filemask
 
 image create bitmap file_fulltick -background white -foreground "#007000" -data {
 #define file_fulltick_width 14
 #define file_fulltick_height 15
 static unsigned char file_fulltick_bits[] = {
-   0xfe, 0x01, 0x02, 0x1a, 0x02, 0x0c, 0x02, 0x0c, 0x02, 0x16, 0x02, 0x16,
-   0x02, 0x13, 0x00, 0x13, 0x86, 0x11, 0x8c, 0x11, 0xd8, 0x10, 0xf2, 0x10,
-   0x62, 0x10, 0x02, 0x10, 0xfe, 0x1f};
+	0xfe, 0x01, 0x02, 0x1a, 0x02, 0x0c, 0x02, 0x0c, 0x02, 0x16, 0x02, 0x16,
+	0x02, 0x13, 0x00, 0x13, 0x86, 0x11, 0x8c, 0x11, 0xd8, 0x10, 0xf2, 0x10,
+	0x62, 0x10, 0x02, 0x10, 0xfe, 0x1f};
 } -maskdata $filemask
 
 image create bitmap file_question -background white -foreground black -data {
 #define file_question_width 14
 #define file_question_height 15
 static unsigned char file_question_bits[] = {
-   0xfe, 0x01, 0x02, 0x02, 0xe2, 0x04, 0xf2, 0x09, 0x1a, 0x1b, 0x0a, 0x13,
-   0x82, 0x11, 0xc2, 0x10, 0x62, 0x10, 0x62, 0x10, 0x02, 0x10, 0x62, 0x10,
-   0x62, 0x10, 0x02, 0x10, 0xfe, 0x1f};
+	0xfe, 0x01, 0x02, 0x02, 0xe2, 0x04, 0xf2, 0x09, 0x1a, 0x1b, 0x0a, 0x13,
+	0x82, 0x11, 0xc2, 0x10, 0x62, 0x10, 0x62, 0x10, 0x02, 0x10, 0x62, 0x10,
+	0x62, 0x10, 0x02, 0x10, 0xfe, 0x1f};
 } -maskdata $filemask
 
 image create bitmap file_removed -background white -foreground red -data {
 #define file_removed_width 14
 #define file_removed_height 15
 static unsigned char file_removed_bits[] = {
-   0xfe, 0x01, 0x02, 0x03, 0x02, 0x05, 0x02, 0x09, 0x02, 0x1f, 0x02, 0x10,
-   0x1a, 0x16, 0x32, 0x13, 0xe2, 0x11, 0xc2, 0x10, 0xe2, 0x11, 0x32, 0x13,
-   0x1a, 0x16, 0x02, 0x10, 0xfe, 0x1f};
+	0xfe, 0x01, 0x02, 0x03, 0x02, 0x05, 0x02, 0x09, 0x02, 0x1f, 0x02, 0x10,
+	0x1a, 0x16, 0x32, 0x13, 0xe2, 0x11, 0xc2, 0x10, 0xe2, 0x11, 0x32, 0x13,
+	0x1a, 0x16, 0x02, 0x10, 0xfe, 0x1f};
 } -maskdata $filemask
 
 image create bitmap file_merge -background white -foreground blue -data {
 #define file_merge_width 14
 #define file_merge_height 15
 static unsigned char file_merge_bits[] = {
-   0xfe, 0x01, 0x02, 0x03, 0x62, 0x05, 0x62, 0x09, 0x62, 0x1f, 0x62, 0x10,
-   0xfa, 0x11, 0xf2, 0x10, 0x62, 0x10, 0x02, 0x10, 0xfa, 0x17, 0x02, 0x10,
-   0xfa, 0x17, 0x02, 0x10, 0xfe, 0x1f};
+	0xfe, 0x01, 0x02, 0x03, 0x62, 0x05, 0x62, 0x09, 0x62, 0x1f, 0x62, 0x10,
+	0xfa, 0x11, 0xf2, 0x10, 0x62, 0x10, 0x02, 0x10, 0xfa, 0x17, 0x02, 0x10,
+	0xfa, 0x17, 0x02, 0x10, 0xfe, 0x1f};
 } -maskdata $filemask
 
 image create bitmap file_statechange -background white -foreground green -data {
 #define file_statechange_width 14
 #define file_statechange_height 15
 static unsigned char file_statechange_bits[] = {
-   0xfe, 0x01, 0x02, 0x03, 0x02, 0x05, 0x02, 0x09, 0x02, 0x1f, 0x62, 0x10,
-   0x62, 0x10, 0xba, 0x11, 0xba, 0x11, 0x62, 0x10, 0x62, 0x10, 0x02, 0x10,
-   0x02, 0x10, 0x02, 0x10, 0xfe, 0x1f};
+	0xfe, 0x01, 0x02, 0x03, 0x02, 0x05, 0x02, 0x09, 0x02, 0x1f, 0x62, 0x10,
+	0x62, 0x10, 0xba, 0x11, 0xba, 0x11, 0x62, 0x10, 0x62, 0x10, 0x02, 0x10,
+	0x02, 0x10, 0x02, 0x10, 0xfe, 0x1f};
 } -maskdata $filemask
 
 set ui_index .vpane.files.index.list
@@ -2088,36 +2086,36 @@ set all_icons(T$ui_workdir) file_statechange
 
 set max_status_desc 0
 foreach i {
-		{__ {mc "Unmodified"}}
-
-		{_M {mc "Modified, not staged"}}
-		{M_ {mc "Staged for commit"}}
-		{MM {mc "Portions staged for commit"}}
-		{MD {mc "Staged for commit, missing"}}
-
-		{_T {mc "File type changed, not staged"}}
-		{MT {mc "File type changed, old type staged for commit"}}
-		{AT {mc "File type changed, old type staged for commit"}}
-		{T_ {mc "File type changed, staged"}}
-		{TM {mc "File type change staged, modification not staged"}}
-		{TD {mc "File type change staged, file missing"}}
-
-		{_O {mc "Untracked, not staged"}}
-		{A_ {mc "Staged for commit"}}
-		{AM {mc "Portions staged for commit"}}
-		{AD {mc "Staged for commit, missing"}}
-
-		{_D {mc "Missing"}}
-		{D_ {mc "Staged for removal"}}
-		{DO {mc "Staged for removal, still present"}}
-
-		{_U {mc "Requires merge resolution"}}
-		{U_ {mc "Requires merge resolution"}}
-		{UU {mc "Requires merge resolution"}}
-		{UM {mc "Requires merge resolution"}}
-		{UD {mc "Requires merge resolution"}}
-		{UT {mc "Requires merge resolution"}}
-	} {
+	{__ {mc "Unmodified"}}
+
+	{_M {mc "Modified, not staged"}}
+	{M_ {mc "Staged for commit"}}
+	{MM {mc "Portions staged for commit"}}
+	{MD {mc "Staged for commit, missing"}}
+
+	{_T {mc "File type changed, not staged"}}
+	{MT {mc "File type changed, old type staged for commit"}}
+	{AT {mc "File type changed, old type staged for commit"}}
+	{T_ {mc "File type changed, staged"}}
+	{TM {mc "File type change staged, modification not staged"}}
+	{TD {mc "File type change staged, file missing"}}
+
+	{_O {mc "Untracked, not staged"}}
+	{A_ {mc "Staged for commit"}}
+	{AM {mc "Portions staged for commit"}}
+	{AD {mc "Staged for commit, missing"}}
+
+	{_D {mc "Missing"}}
+	{D_ {mc "Staged for removal"}}
+	{DO {mc "Staged for removal, still present"}}
+
+	{_U {mc "Requires merge resolution"}}
+	{U_ {mc "Requires merge resolution"}}
+	{UU {mc "Requires merge resolution"}}
+	{UM {mc "Requires merge resolution"}}
+	{UD {mc "Requires merge resolution"}}
+	{UT {mc "Requires merge resolution"}}
+} {
 	set text [eval [lindex $i 1]]
 	if {$max_status_desc < [string length $text]} {
 		set max_status_desc [string length $text]
@@ -2475,8 +2473,10 @@ proc next_diff_after_action {w path {lno {}} {mmask {}}} {
 proc select_first_diff {after} {
 	global ui_workdir
 
-	if {[find_next_diff $ui_workdir {} 1 {^_?U}] ||
-	    [find_next_diff $ui_workdir {} 1 {[^O]$}]} {
+	if {
+		[find_next_diff $ui_workdir {} 1 {^_?U}]
+		|| [find_next_diff $ui_workdir {} 1 {[^O]$}]
+	} {
 		next_diff $after
 	} else {
 		uplevel #0 $after
@@ -3111,8 +3111,11 @@ proc normalize_relpath {path} {
 	set elements {}
 	foreach item [file split $path] {
 		if {$item eq {.}} continue
-		if {$item eq {..} && [llength $elements] > 0
-		    && [lindex $elements end] ne {..}} {
+		if {
+			$item eq {..}
+			&& [llength $elements] > 0
+			&& [lindex $elements end] ne {..}
+		} {
 			set elements [lrange $elements 0 end-1]
 			continue
 		}
@@ -3878,18 +3881,18 @@ proc on_application_mapped {} {
 	set gm $repo_config(gui.geometry)
 	if {$use_ttk} {
 		bind .vpane <Map> \
-		    [list on_ttk_pane_mapped %W 0 [lindex $gm 1]]
+			[list on_ttk_pane_mapped %W 0 [lindex $gm 1]]
 		bind .vpane.files <Map> \
-		    [list on_ttk_pane_mapped %W 0 [lindex $gm 2]]
+			[list on_ttk_pane_mapped %W 0 [lindex $gm 2]]
 	} else {
 		bind .vpane <Map> \
-		    [list on_tk_pane_mapped %W 0 \
-			 [lindex $gm 1] \
-			 [lindex [.vpane sash coord 0] 1]]
+			[list on_tk_pane_mapped %W 0 \
+			[lindex $gm 1] \
+			[lindex [.vpane sash coord 0] 1]]
 		bind .vpane.files <Map> \
-		    [list on_tk_pane_mapped %W 0 \
-			 [lindex [.vpane.files sash coord 0] 0] \
-			 [lindex $gm 2]]
+			[list on_tk_pane_mapped %W 0 \
+			[lindex [.vpane.files sash coord 0] 0] \
+			[lindex $gm 2]]
 	}
 	wm geometry . [lindex $gm 0]
 }
@@ -4145,8 +4148,10 @@ if {[winfo exists $ui_comm]} {
 	lappend spell_cmd --mode=none
 	lappend spell_cmd --encoding=utf-8
 	lappend spell_cmd pipe
-	if {$spell_dict eq {none}
-	 || [catch {set spell_fd [open $spell_cmd r+]} spell_err]} {
+	if {
+		$spell_dict eq {none}
+		|| [catch {set spell_fd [open $spell_cmd r+]} spell_err]
+	} {
 		bind_button3 $ui_comm [list tk_popup $ui_comm_ctxm %X %Y]
 	} else {
 		set ui_comm_spell [spellcheck::init \

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

* Re: style(git-gui): Fix mixed tabs & spaces; Always use tabs.
  2020-08-22 10:56 style(git-gui): Fix mixed tabs & spaces; Always use tabs Serg Tereshchenko
@ 2020-08-22 19:26 ` Junio C Hamano
  2020-08-22 22:24   ` [PATCH v2] style(git-gui): Fix mixed tabs & spaces; Prefer tabs Serg Tereshchenko
  0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2020-08-22 19:26 UTC (permalink / raw)
  To: Serg Tereshchenko; +Cc: git

Serg Tereshchenko <serg.partizan@gmail.com> writes:

> Hello.
>
> I want to improve styling of git-citool, so it supports dark themes.
> But first i want to remove "mixed indent warning" from my editor
> modeline.
>
> This patch does not change anything besides indents, and sometimes i
> replace
>
> ```tcl
> if {long ||
>     multiline ||
>     statement}
> ```
>
> with more readable
>
> ```tcl
> if {
>     long
>     || multiline
>     || statement
> }
> ```
> But only in lines with mixed indents.

You don't ever want to do that in a patch for "fix indentation"
patch.

Besides, the comparison between these two styles is subjective, and
you do not represent the majority view of who needs to work on the
code in git-gui, so "with more readable" is not a very good
justification to do so.

If the existing code in the whole file uses mixture of both styles,
and one style is used overwhelmingly more than the other, a
follow-up patch to adjust the style to one, with "consistency" as
the justification, would be a good idea, though.

Thanks.

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

* [PATCH v2] style(git-gui): Fix mixed tabs & spaces; Prefer tabs.
  2020-08-22 19:26 ` Junio C Hamano
@ 2020-08-22 22:24   ` Serg Tereshchenko
  2020-09-09  4:51     ` Pratyush Yadav
  0 siblings, 1 reply; 6+ messages in thread
From: Serg Tereshchenko @ 2020-08-22 22:24 UTC (permalink / raw)
  To: git, gitster; +Cc: Serg Tereshchenko

Here is cleaned up version of the patch.

Spaces replaced with tabs when possible. In some cases just replacing
spaces with tabs would break readability, so it was left as it is.

Signed-off-by: Serg Tereshchenko <serg.partizan@gmail.com>
---
 git-gui.sh | 154 ++++++++++++++++++++++++++---------------------------
 1 file changed, 77 insertions(+), 77 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index 49bd86e..847c3c9 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -947,15 +947,15 @@ if {![regsub {^git version } $_git_version {} _git_version]} {
 }
 
 proc get_trimmed_version {s} {
-    set r {}
-    foreach x [split $s -._] {
-        if {[string is integer -strict $x]} {
-            lappend r $x
-        } else {
-            break
-        }
-    }
-    return [join $r .]
+	set r {}
+	foreach x [split $s -._] {
+		if {[string is integer -strict $x]} {
+			lappend r $x
+		} else {
+			break
+		}
+	}
+	return [join $r .]
 }
 set _real_git_version $_git_version
 set _git_version [get_trimmed_version $_git_version]
@@ -967,7 +967,7 @@ if {![regexp {^[1-9]+(\.[0-9]+)+$} $_git_version]} {
 		-type yesno \
 		-default no \
 		-title "[appname]: warning" \
-		 -message [mc "Git version cannot be determined.
+		-message [mc "Git version cannot be determined.
 
 %s claims it is version '%s'.
 
@@ -1181,44 +1181,44 @@ enable_option transport
 disable_option bare
 
 switch -- $subcommand {
-browser -
-blame {
-	enable_option bare
-
-	disable_option multicommit
-	disable_option branch
-	disable_option transport
-}
-citool {
-	enable_option singlecommit
-	enable_option retcode
-
-	disable_option multicommit
-	disable_option branch
-	disable_option transport
+	browser -
+	blame {
+		enable_option bare
+
+		disable_option multicommit
+		disable_option branch
+		disable_option transport
+	}
+	citool {
+		enable_option singlecommit
+		enable_option retcode
+
+		disable_option multicommit
+		disable_option branch
+		disable_option transport
+
+		while {[llength $argv] > 0} {
+			set a [lindex $argv 0]
+			switch -- $a {
+				--amend {
+					enable_option initialamend
+				}
+				--nocommit {
+					enable_option nocommit
+					enable_option nocommitmsg
+				}
+				--commitmsg {
+					disable_option nocommitmsg
+				}
+				default {
+					break
+				}
+			}
 
-	while {[llength $argv] > 0} {
-		set a [lindex $argv 0]
-		switch -- $a {
-		--amend {
-			enable_option initialamend
-		}
-		--nocommit {
-			enable_option nocommit
-			enable_option nocommitmsg
+			set argv [lrange $argv 1 end]
 		}
-		--commitmsg {
-			disable_option nocommitmsg
-		}
-		default {
-			break
-		}
-		}
-
-		set argv [lrange $argv 1 end]
 	}
 }
-}
 
 ######################################################################
 ##
@@ -1653,7 +1653,7 @@ proc prepare_commit_msg_hook_wait {fd_ph} {
 		set pch_error {}
 		catch {file delete [gitdir PREPARE_COMMIT_MSG]}
 		return
-        }
+	}
 	fconfigure $fd_ph -blocking 0
 	catch {file delete [gitdir PREPARE_COMMIT_MSG]}
 }
@@ -2001,72 +2001,72 @@ set filemask {
 #define mask_width 14
 #define mask_height 15
 static unsigned char mask_bits[] = {
-   0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f,
-   0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f,
-   0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f};
+	0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f,
+	0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f,
+	0xfe, 0x1f, 0xfe, 0x1f, 0xfe, 0x1f};
 }
 
 image create bitmap file_plain -background white -foreground black -data {
 #define plain_width 14
 #define plain_height 15
 static unsigned char plain_bits[] = {
-   0xfe, 0x01, 0x02, 0x03, 0x02, 0x05, 0x02, 0x09, 0x02, 0x1f, 0x02, 0x10,
-   0x02, 0x10, 0x02, 0x10, 0x02, 0x10, 0x02, 0x10, 0x02, 0x10, 0x02, 0x10,
-   0x02, 0x10, 0x02, 0x10, 0xfe, 0x1f};
+	0xfe, 0x01, 0x02, 0x03, 0x02, 0x05, 0x02, 0x09, 0x02, 0x1f, 0x02, 0x10,
+	0x02, 0x10, 0x02, 0x10, 0x02, 0x10, 0x02, 0x10, 0x02, 0x10, 0x02, 0x10,
+	0x02, 0x10, 0x02, 0x10, 0xfe, 0x1f};
 } -maskdata $filemask
 
 image create bitmap file_mod -background white -foreground blue -data {
 #define mod_width 14
 #define mod_height 15
 static unsigned char mod_bits[] = {
-   0xfe, 0x01, 0x02, 0x03, 0x7a, 0x05, 0x02, 0x09, 0x7a, 0x1f, 0x02, 0x10,
-   0xfa, 0x17, 0x02, 0x10, 0xfa, 0x17, 0x02, 0x10, 0xfa, 0x17, 0x02, 0x10,
-   0xfa, 0x17, 0x02, 0x10, 0xfe, 0x1f};
+	0xfe, 0x01, 0x02, 0x03, 0x7a, 0x05, 0x02, 0x09, 0x7a, 0x1f, 0x02, 0x10,
+	0xfa, 0x17, 0x02, 0x10, 0xfa, 0x17, 0x02, 0x10, 0xfa, 0x17, 0x02, 0x10,
+	0xfa, 0x17, 0x02, 0x10, 0xfe, 0x1f};
 } -maskdata $filemask
 
 image create bitmap file_fulltick -background white -foreground "#007000" -data {
 #define file_fulltick_width 14
 #define file_fulltick_height 15
 static unsigned char file_fulltick_bits[] = {
-   0xfe, 0x01, 0x02, 0x1a, 0x02, 0x0c, 0x02, 0x0c, 0x02, 0x16, 0x02, 0x16,
-   0x02, 0x13, 0x00, 0x13, 0x86, 0x11, 0x8c, 0x11, 0xd8, 0x10, 0xf2, 0x10,
-   0x62, 0x10, 0x02, 0x10, 0xfe, 0x1f};
+	0xfe, 0x01, 0x02, 0x1a, 0x02, 0x0c, 0x02, 0x0c, 0x02, 0x16, 0x02, 0x16,
+	0x02, 0x13, 0x00, 0x13, 0x86, 0x11, 0x8c, 0x11, 0xd8, 0x10, 0xf2, 0x10,
+	0x62, 0x10, 0x02, 0x10, 0xfe, 0x1f};
 } -maskdata $filemask
 
 image create bitmap file_question -background white -foreground black -data {
 #define file_question_width 14
 #define file_question_height 15
 static unsigned char file_question_bits[] = {
-   0xfe, 0x01, 0x02, 0x02, 0xe2, 0x04, 0xf2, 0x09, 0x1a, 0x1b, 0x0a, 0x13,
-   0x82, 0x11, 0xc2, 0x10, 0x62, 0x10, 0x62, 0x10, 0x02, 0x10, 0x62, 0x10,
-   0x62, 0x10, 0x02, 0x10, 0xfe, 0x1f};
+	0xfe, 0x01, 0x02, 0x02, 0xe2, 0x04, 0xf2, 0x09, 0x1a, 0x1b, 0x0a, 0x13,
+	0x82, 0x11, 0xc2, 0x10, 0x62, 0x10, 0x62, 0x10, 0x02, 0x10, 0x62, 0x10,
+	0x62, 0x10, 0x02, 0x10, 0xfe, 0x1f};
 } -maskdata $filemask
 
 image create bitmap file_removed -background white -foreground red -data {
 #define file_removed_width 14
 #define file_removed_height 15
 static unsigned char file_removed_bits[] = {
-   0xfe, 0x01, 0x02, 0x03, 0x02, 0x05, 0x02, 0x09, 0x02, 0x1f, 0x02, 0x10,
-   0x1a, 0x16, 0x32, 0x13, 0xe2, 0x11, 0xc2, 0x10, 0xe2, 0x11, 0x32, 0x13,
-   0x1a, 0x16, 0x02, 0x10, 0xfe, 0x1f};
+	0xfe, 0x01, 0x02, 0x03, 0x02, 0x05, 0x02, 0x09, 0x02, 0x1f, 0x02, 0x10,
+	0x1a, 0x16, 0x32, 0x13, 0xe2, 0x11, 0xc2, 0x10, 0xe2, 0x11, 0x32, 0x13,
+	0x1a, 0x16, 0x02, 0x10, 0xfe, 0x1f};
 } -maskdata $filemask
 
 image create bitmap file_merge -background white -foreground blue -data {
 #define file_merge_width 14
 #define file_merge_height 15
 static unsigned char file_merge_bits[] = {
-   0xfe, 0x01, 0x02, 0x03, 0x62, 0x05, 0x62, 0x09, 0x62, 0x1f, 0x62, 0x10,
-   0xfa, 0x11, 0xf2, 0x10, 0x62, 0x10, 0x02, 0x10, 0xfa, 0x17, 0x02, 0x10,
-   0xfa, 0x17, 0x02, 0x10, 0xfe, 0x1f};
+	0xfe, 0x01, 0x02, 0x03, 0x62, 0x05, 0x62, 0x09, 0x62, 0x1f, 0x62, 0x10,
+	0xfa, 0x11, 0xf2, 0x10, 0x62, 0x10, 0x02, 0x10, 0xfa, 0x17, 0x02, 0x10,
+	0xfa, 0x17, 0x02, 0x10, 0xfe, 0x1f};
 } -maskdata $filemask
 
 image create bitmap file_statechange -background white -foreground green -data {
 #define file_statechange_width 14
 #define file_statechange_height 15
 static unsigned char file_statechange_bits[] = {
-   0xfe, 0x01, 0x02, 0x03, 0x02, 0x05, 0x02, 0x09, 0x02, 0x1f, 0x62, 0x10,
-   0x62, 0x10, 0xba, 0x11, 0xba, 0x11, 0x62, 0x10, 0x62, 0x10, 0x02, 0x10,
-   0x02, 0x10, 0x02, 0x10, 0xfe, 0x1f};
+	0xfe, 0x01, 0x02, 0x03, 0x02, 0x05, 0x02, 0x09, 0x02, 0x1f, 0x62, 0x10,
+	0x62, 0x10, 0xba, 0x11, 0xba, 0x11, 0x62, 0x10, 0x62, 0x10, 0x02, 0x10,
+	0x02, 0x10, 0x02, 0x10, 0xfe, 0x1f};
 } -maskdata $filemask
 
 set ui_index .vpane.files.index.list
@@ -3878,18 +3878,18 @@ proc on_application_mapped {} {
 	set gm $repo_config(gui.geometry)
 	if {$use_ttk} {
 		bind .vpane <Map> \
-		    [list on_ttk_pane_mapped %W 0 [lindex $gm 1]]
+			[list on_ttk_pane_mapped %W 0 [lindex $gm 1]]
 		bind .vpane.files <Map> \
-		    [list on_ttk_pane_mapped %W 0 [lindex $gm 2]]
+			[list on_ttk_pane_mapped %W 0 [lindex $gm 2]]
 	} else {
 		bind .vpane <Map> \
-		    [list on_tk_pane_mapped %W 0 \
-			 [lindex $gm 1] \
-			 [lindex [.vpane sash coord 0] 1]]
+			[list on_tk_pane_mapped %W 0 \
+			[lindex $gm 1] \
+			[lindex [.vpane sash coord 0] 1]]
 		bind .vpane.files <Map> \
-		    [list on_tk_pane_mapped %W 0 \
-			 [lindex [.vpane.files sash coord 0] 0] \
-			 [lindex $gm 2]]
+			[list on_tk_pane_mapped %W 0 \
+			[lindex [.vpane.files sash coord 0] 0] \
+			[lindex $gm 2]]
 	}
 	wm geometry . [lindex $gm 0]
 }
-- 
2.28.0


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

* Re: [PATCH v2] style(git-gui): Fix mixed tabs & spaces; Prefer tabs.
  2020-08-22 22:24   ` [PATCH v2] style(git-gui): Fix mixed tabs & spaces; Prefer tabs Serg Tereshchenko
@ 2020-09-09  4:51     ` Pratyush Yadav
  2020-09-09 13:01       ` Serg Tereshchenko
  0 siblings, 1 reply; 6+ messages in thread
From: Pratyush Yadav @ 2020-09-09  4:51 UTC (permalink / raw)
  To: Serg Tereshchenko; +Cc: git, gitster

Hi Serg,

Thanks for the patch.

> Subject: [PATCH v2] style(git-gui): Fix mixed tabs & spaces; Prefer tabs.

s/style(git-gui)/git-gui/

On 23/08/20 01:24AM, Serg Tereshchenko wrote:
> Here is cleaned up version of the patch.

A line like this should not be a part of the actual commit message. It 
is some extra commentary for the reviewers. The way you have submitted 
this patch, this line would end up in the commit message. The usual way 
of doing something like this is to use "scissors".

You can add this line:

--- 8< ---

This "scissors" line can tell git-am (when the option --scissors) to 
ignore everything above it. That makes my job a little bit easier when 
applying your patch :-)
 
> Spaces replaced with tabs when possible. In some cases just replacing
> spaces with tabs would break readability, so it was left as it is.
> 
> Signed-off-by: Serg Tereshchenko <serg.partizan@gmail.com>
> ---
>  git-gui.sh | 154 ++++++++++++++++++++++++++---------------------------
>  1 file changed, 77 insertions(+), 77 deletions(-)

Most of the changes here look good to me. One comment below.

> 
> diff --git a/git-gui.sh b/git-gui.sh
> index 49bd86e..847c3c9 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -947,15 +947,15 @@ if {![regsub {^git version } $_git_version {} _git_version]} {
>  }
>  
>  proc get_trimmed_version {s} {
> -    set r {}
> -    foreach x [split $s -._] {
> -        if {[string is integer -strict $x]} {
> -            lappend r $x
> -        } else {
> -            break
> -        }
> -    }
> -    return [join $r .]
> +	set r {}
> +	foreach x [split $s -._] {
> +		if {[string is integer -strict $x]} {
> +			lappend r $x
> +		} else {
> +			break
> +		}
> +	}
> +	return [join $r .]
>  }
>  set _real_git_version $_git_version
>  set _git_version [get_trimmed_version $_git_version]
> @@ -967,7 +967,7 @@ if {![regexp {^[1-9]+(\.[0-9]+)+$} $_git_version]} {
>  		-type yesno \
>  		-default no \
>  		-title "[appname]: warning" \
> -		 -message [mc "Git version cannot be determined.
> +		-message [mc "Git version cannot be determined.
>  
>  %s claims it is version '%s'.
>  
> @@ -1181,44 +1181,44 @@ enable_option transport
>  disable_option bare
>  
>  switch -- $subcommand {
> -browser -
> -blame {
> -	enable_option bare
> -
> -	disable_option multicommit
> -	disable_option branch
> -	disable_option transport
> -}
> -citool {
> -	enable_option singlecommit
> -	enable_option retcode
> -
> -	disable_option multicommit
> -	disable_option branch
> -	disable_option transport
> +	browser -
> +	blame {
> +		enable_option bare
> +
> +		disable_option multicommit
> +		disable_option branch
> +		disable_option transport
> +	}
> +	citool {
> +		enable_option singlecommit
> +		enable_option retcode
> +
> +		disable_option multicommit
> +		disable_option branch
> +		disable_option transport
> +
> +		while {[llength $argv] > 0} {
> +			set a [lindex $argv 0]
> +			switch -- $a {
> +				--amend {
> +					enable_option initialamend
> +				}
> +				--nocommit {
> +					enable_option nocommit
> +					enable_option nocommitmsg
> +				}
> +				--commitmsg {
> +					disable_option nocommitmsg
> +				}
> +				default {
> +					break
> +				}
> +			}
>  
> -	while {[llength $argv] > 0} {
> -		set a [lindex $argv 0]
> -		switch -- $a {
> -		--amend {
> -			enable_option initialamend
> -		}
> -		--nocommit {
> -			enable_option nocommit
> -			enable_option nocommitmsg
> +			set argv [lrange $argv 1 end]
>  		}
> -		--commitmsg {
> -			disable_option nocommitmsg
> -		}
> -		default {
> -			break
> -		}
> -		}
> -
> -		set argv [lrange $argv 1 end]
>  	}
>  }
> -}

I'm not on board with this entire hunk. In many C projects (like Linux, 
Git, etc) the "switch" and the "case" are on the same indent level. I 
can see instances of this in almost every switch-case block in 
git-gui.sh as well. We should stick to the local convention here and 
drop this hunk.

I can make these changes locally and merge them so no need to re-roll... 
unless you have any counter points that is.

-- 
Regards,
Pratyush Yadav

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

* Re: [PATCH v2] style(git-gui): Fix mixed tabs & spaces; Prefer tabs.
  2020-09-09  4:51     ` Pratyush Yadav
@ 2020-09-09 13:01       ` Serg Tereshchenko
  2020-09-22  9:52         ` Pratyush Yadav
  0 siblings, 1 reply; 6+ messages in thread
From: Serg Tereshchenko @ 2020-09-09 13:01 UTC (permalink / raw)
  To: me; +Cc: git, gitster, serg.partizan

Hi Pratyush,

Thanks for suggestion about scissors, i was wondering how to do this
properly, i'll try it next time.

> I'm not on board with this entire hunk. In many C projects (like Linux, 
> Git, etc) the "switch" and the "case" are on the same indent level. I 
> can see instances of this in almost every switch-case block in 
> git-gui.sh as well. We should stick to the local convention here and 
> drop this hunk.
> 
> I can make these changes locally and merge them so no need to re-roll... 
> unless you have any counter points that is.

I have no objections, please drop that hunk.

--
Regards,
Serg Tereshchenko

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

* Re: [PATCH v2] style(git-gui): Fix mixed tabs & spaces; Prefer tabs.
  2020-09-09 13:01       ` Serg Tereshchenko
@ 2020-09-22  9:52         ` Pratyush Yadav
  0 siblings, 0 replies; 6+ messages in thread
From: Pratyush Yadav @ 2020-09-22  9:52 UTC (permalink / raw)
  To: Serg Tereshchenko; +Cc: git, gitster

On 09/09/20 04:01PM, Serg Tereshchenko wrote:
> Hi Pratyush,
> 
> Thanks for suggestion about scissors, i was wondering how to do this
> properly, i'll try it next time.
> 
> > I'm not on board with this entire hunk. In many C projects (like Linux, 
> > Git, etc) the "switch" and the "case" are on the same indent level. I 
> > can see instances of this in almost every switch-case block in 
> > git-gui.sh as well. We should stick to the local convention here and 
> > drop this hunk.
> > 
> > I can make these changes locally and merge them so no need to re-roll... 
> > unless you have any counter points that is.
> 
> I have no objections, please drop that hunk.

Merged with the above changes. Thanks.
 
> --
> Regards,
> Serg Tereshchenko

-- 
Regards,
Pratyush Yadav

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

end of thread, other threads:[~2020-09-22 10:02 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-22 10:56 style(git-gui): Fix mixed tabs & spaces; Always use tabs Serg Tereshchenko
2020-08-22 19:26 ` Junio C Hamano
2020-08-22 22:24   ` [PATCH v2] style(git-gui): Fix mixed tabs & spaces; Prefer tabs Serg Tereshchenko
2020-09-09  4:51     ` Pratyush Yadav
2020-09-09 13:01       ` Serg Tereshchenko
2020-09-22  9:52         ` Pratyush Yadav

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).