Skip to contentSkip to conversation

gitk: expand $config_file_tmp before reporting to user

PATCH In review Max Kirillov 4 participants 7 messages Sep 28, 2017 – Sep 29, 2017

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 gitk: expand $config_file_tmp before reporting to user

Tilda-based path may confise some users. First, tilda is not known for Window users, second, it may point to unexpected location depending on various environment setup.

Expand the path to "nativename", so that ~/.config/git/gitk-tmp would be "C:\Users\user\.config\git\gitk-tmp", for example. It should be less cryptic

---
 gitk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
gitk +2 -1
index 877d49e1de..48c1d3e714 100755
@@ -2844,7 +2844,8 @@ proc config_check_tmp_exists {tries_left} {
 	if {$tries_left > 0} {
 	    after 100 [list config_check_tmp_exists $tries_left]
 	} else {
-	    error_popup "There appears to be a stale $config_file_tmp\
+	    error_popup "There appears to be a stale \
+ \"[file nativename $config_file_tmp]\" \
  file, which will prevent gitk from saving its configuration on exit.\
  Please remove it if it is not being used by any existing gitk process."
 	}
Signature · 1 line
2.11.0.1122.gc3fec58.dirty
#1 lore raw
Quoted 7 lines · Max Kirillov

Tilda-based path may confise some users. First, tilda is not known for Window users, second, it may point to unexpected location depending on various environment setup.

Expand the path to "nativename", so that ~/.config/git/gitk-tmp would be "C:\Users\user\.config\git\gitk-tmp", for example. It should be less cryptic

It might be less cryptic, but for those of us whose $HOME is a looooooong path, ~/.config/git/gitk-tmp is much easier to understand than the same path with ~/ expanded, which would push the part of the filename that most matters far to the right hand side of the dialog.

I somehow find this change just robbing Peter to pay Paul.

Quoted 4 lines
gitk +2 -1
 	} else {
-	    error_popup "There appears to be a stale $config_file_tmp\
+	    error_popup "There appears to be a stale \
+ \"[file nativename $config_file_tmp]\" \
#2 lore raw
Quoted 10 lines · Junio C Hamano
Quoted 7 lines · Max Kirillov

Tilda-based path may confise some users. First, tilda is not known for Window users, second, it may point to unexpected location depending on various environment setup.

Expand the path to "nativename", so that ~/.config/git/gitk-tmp would be "C:\Users\user\.config\git\gitk-tmp", for example. It should be less cryptic

Quoted 7 lines

It might be less cryptic, but for those of us whose $HOME is a looooooong path, ~/.config/git/gitk-tmp is much easier to understand than the same path with ~/ expanded, which would push the part of the filename that most matters far to the right hand side of the dialog.

I somehow find this change just robbing Peter to pay Paul.

Having said that, because a set-up might have HOME or XDG_CONFIG or other things misconfigured to point at a place where the end user may not be expecting, I tend to think that catering to Paul by showing the information closer to the bare metal is much more worthy thing to do than keeping Peter happy. Since this is an error path, accuracy trumps convenience.

So no objection from me (unless somebody else comes up with an alternative that would make both camps happy, that is).

Thanks.

#3 lore raw

Hi Junio,

Quoted 11 lines · Junio C Hamano
Quoted 9 lines · Junio C Hamano
Quoted 7 lines · Max Kirillov

Tilda-based path may confise some users. First, tilda is not known for Window users, second, it may point to unexpected location depending on various environment setup.

Expand the path to "nativename", so that ~/.config/git/gitk-tmp would be "C:\Users\user\.config\git\gitk-tmp", for example. It should be less cryptic

Thanks, Max, for your contribution!

Quoted 5 lines
Quoted 5 lines

It might be less cryptic, but for those of us whose $HOME is a looooooong path, ~/.config/git/gitk-tmp is much easier to understand than the same path with ~/ expanded, which would push the part of the filename that most matters far to the right hand side of the dialog.

Heh, do you want to know how that must sound to a Windows user? I'm not saying that I am a hard-core Windows user, but I do know a few, and I already hear their comments in their own voice in my head...

Quoted 11 lines
Quoted 1 line

I somehow find this change just robbing Peter to pay Paul.

Having said that, because a set-up might have HOME or XDG_CONFIG or other things misconfigured to point at a place where the end user may not be expecting, I tend to think that catering to Paul by showing the information closer to the bare metal is much more worthy thing to do than keeping Peter happy. Since this is an error path, accuracy trumps convenience.

So no objection from me (unless somebody else comes up with an alternative that would make both camps happy, that is).

To Unix/Linux users, the tilde (or Tilda, I really like that nickname, it makes it much more human and tolerable) is probably *very* familiar.

As familiar, as it is unfamiliar to Windows users.

So I would actually suggest to make this a conditional on the platform: on Windows, use the native name, everywhere else, not.

Sound good?
Johannes

#4 lore raw
Quoted 5 lines · Johannes Schindelin
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines · Max Kirillov

Tilda-based path may confise some users. First, tilda is not known for Window users, second, it may point to unexpected location depending on various environment setup.

Quoted 7 lines

...
As familiar, as it is unfamiliar to Windows users.

So I would actually suggest to make this a conditional on the platform: on Windows, use the native name, everywhere else, not.

Sound good?

Not really.

I agree with and (more importantly) consider the second rationale Max cites a more relevant one for this change.

This is about reporting an error, and using the short-hand ~/$rest that could be pointing at a location different from what the user _thinks_ it points at for whatever reason (miconfigured HOME, some intermediate process setting it to something else, etc.) can hide the real issue. The problem can be more easily noticed and diagnosed if the message shows the result of 'nativename'.

And that rationale holds whether you are seeing the error message on Windows or non-Windows.

#5 lore raw
In reply to Johannes Schindelin
Quoted 10 lines · Johannes Schindelin
Quoted 8 lines
Quoted 8 lines
Quoted 7 lines · Max Kirillov

Tilda-based path may confise some users. First, tilda is not known for Window users, second, it may point to unexpected location depending on various environment setup.

Expand the path to "nativename", so that ~/.config/git/gitk-tmp would be "C:\Users\user\.config\git\gitk-tmp", for example. It should be less cryptic

Thanks, Max, for your contribution!

I do what I can. Just noticed s question at SO about it
(https://stackoverflow.com/questions/46450479/how-to-remove-the-stale-gitk-tmp-file)
Provided that I was author of the message, it looked like something for me to fix.

Quoted 1 line

Sound good?

As Junio noticed, it would be more reliable to show full path, and error message does not have to be very nice anyway. Also, gitk is already too big and I always feel bad when adding stuff to it, so let's save couple of lines by not adding another "if".

Signature · 1 line
Max
#6 lore raw
In reply to Junio C Hamano

Well something like this is not paying _Paul_ enough for what he gave to The People, so I do not think there is worth trying.

Quoted 25 lines · Junio C Hamano
Quoted 7 lines · Max Kirillov

Tilda-based path may confise some users. First, tilda is not known for Window users, second, it may point to unexpected location depending on various environment setup.

Expand the path to "nativename", so that ~/.config/git/gitk-tmp would be "C:\Users\user\.config\git\gitk-tmp", for example. It should be less cryptic

It might be less cryptic, but for those of us whose $HOME is a looooooong path, ~/.config/git/gitk-tmp is much easier to understand than the same path with ~/ expanded, which would push the part of the filename that most matters far to the right hand side of the dialog.

I somehow find this change just robbing Peter to pay Paul.

Quoted 4 lines
	} else {
gitk +2 -1
-	    error_popup "There appears to be a stale $config_file_tmp\
+	    error_popup "There appears to be a stale \
+ \"[file nativename $config_file_tmp]\" \