git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] gitk: expand $config_file_tmp before reporting to user
@ 2017-09-28  4:14 Max Kirillov
  2017-09-28  4:37 ` Junio C Hamano
  0 siblings, 1 reply; 7+ messages in thread
From: Max Kirillov @ 2017-09-28  4:14 UTC (permalink / raw)
  To: Paul Mackerras, Johannes Schindelin; +Cc: Max Kirillov, Junio C Hamano, git

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

Signed-off-by: Max Kirillov <max@max630.net>
---
 gitk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gitk b/gitk
index 877d49e1de..48c1d3e714 100755
--- a/gitk
+++ b/gitk
@@ -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."
 	}
-- 
2.11.0.1122.gc3fec58.dirty


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

* Re: [PATCH] gitk: expand $config_file_tmp before reporting to user
  2017-09-28  4:14 [PATCH] gitk: expand $config_file_tmp before reporting to user Max Kirillov
@ 2017-09-28  4:37 ` Junio C Hamano
  2017-09-28  4:47   ` Junio C Hamano
  2017-09-29 20:24   ` Uxío Prego
  0 siblings, 2 replies; 7+ messages in thread
From: Junio C Hamano @ 2017-09-28  4:37 UTC (permalink / raw)
  To: Max Kirillov; +Cc: Paul Mackerras, Johannes Schindelin, git

Max Kirillov <max@max630.net> writes:

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

>  	} 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]\" \


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

* Re: [PATCH] gitk: expand $config_file_tmp before reporting to user
  2017-09-28  4:37 ` Junio C Hamano
@ 2017-09-28  4:47   ` Junio C Hamano
  2017-09-28 12:31     ` Johannes Schindelin
  2017-09-29 20:24   ` Uxío Prego
  1 sibling, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2017-09-28  4:47 UTC (permalink / raw)
  To: Max Kirillov; +Cc: Paul Mackerras, Johannes Schindelin, git

Junio C Hamano <gitster@pobox.com> writes:

> Max Kirillov <max@max630.net> writes:
>
>> 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.

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.


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

* Re: [PATCH] gitk: expand $config_file_tmp before reporting to user
  2017-09-28  4:47   ` Junio C Hamano
@ 2017-09-28 12:31     ` Johannes Schindelin
  2017-09-28 22:03       ` Junio C Hamano
  2017-09-29  4:34       ` Max Kirillov
  0 siblings, 2 replies; 7+ messages in thread
From: Johannes Schindelin @ 2017-09-28 12:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Max Kirillov, Paul Mackerras, git

Hi Junio,

On Thu, 28 Sep 2017, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Max Kirillov <max@max630.net> writes:
> >
> >> 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!

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

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

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

* Re: [PATCH] gitk: expand $config_file_tmp before reporting to user
  2017-09-28 12:31     ` Johannes Schindelin
@ 2017-09-28 22:03       ` Junio C Hamano
  2017-09-29  4:34       ` Max Kirillov
  1 sibling, 0 replies; 7+ messages in thread
From: Junio C Hamano @ 2017-09-28 22:03 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Max Kirillov, Paul Mackerras, git

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> > Max Kirillov <max@max630.net> writes:
>> >
>> >> 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.
> ...
> 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.


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

* Re: [PATCH] gitk: expand $config_file_tmp before reporting to user
  2017-09-28 12:31     ` Johannes Schindelin
  2017-09-28 22:03       ` Junio C Hamano
@ 2017-09-29  4:34       ` Max Kirillov
  1 sibling, 0 replies; 7+ messages in thread
From: Max Kirillov @ 2017-09-29  4:34 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, Max Kirillov, Paul Mackerras, git

On Thu, Sep 28, 2017 at 02:31:17PM +0200, Johannes Schindelin wrote:
>>> Max Kirillov <max@max630.net> writes:
>>>> 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.

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

-- 
Max

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

* Re: [PATCH] gitk: expand $config_file_tmp before reporting to user
  2017-09-28  4:37 ` Junio C Hamano
  2017-09-28  4:47   ` Junio C Hamano
@ 2017-09-29 20:24   ` Uxío Prego
  1 sibling, 0 replies; 7+ messages in thread
From: Uxío Prego @ 2017-09-29 20:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Max Kirillov, Paul Mackerras, Johannes Schindelin, git

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.

> On 28 Sep 2017, at 06:37, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Max Kirillov <max@max630.net> writes:
> 
>> 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.
> 
>> 	} 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]\" \
> 


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

end of thread, other threads:[~2017-09-29 20:24 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-28  4:14 [PATCH] gitk: expand $config_file_tmp before reporting to user Max Kirillov
2017-09-28  4:37 ` Junio C Hamano
2017-09-28  4:47   ` Junio C Hamano
2017-09-28 12:31     ` Johannes Schindelin
2017-09-28 22:03       ` Junio C Hamano
2017-09-29  4:34       ` Max Kirillov
2017-09-29 20:24   ` Uxío Prego

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