{"thread":{"id":"46851","subject":"[PATCH] gitk: expand $config_file_tmp before reporting to user","startedAt":"2017-09-28T04:14:37Z","lastAt":"2017-09-29T20:24:19Z","messageCount":7,"participants":["Max Kirillov","Junio C Hamano","Johannes Schindelin","Uxío Prego"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"329090","messageId":"20170928041417.28947-1-max@max630.net","threadId":"46851","inReplyTo":null,"subject":"[PATCH] gitk: expand $config_file_tmp before reporting to user","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2017-09-28T04:14:17Z","receivedAt":"2017-09-28T04:14:37Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"Tilda-based path may confise some users. First, tilda is not known\nfor Window users, second, it may point to unexpected location\ndepending on various environment setup.\n\nExpand the path to \"nativename\", so that ~/.config/git/gitk-tmp\nwould be \"C:\\Users\\user\\.config\\git\\gitk-tmp\", for example.\nIt should be less cryptic\n\nSigned-off-by: Max Kirillov <max@max630.net>\n---\n gitk | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/gitk b/gitk\nindex 877d49e1de..48c1d3e714 100755\n--- a/gitk\n+++ b/gitk\n@@ -2844,7 +2844,8 @@ proc config_check_tmp_exists {tries_left} {\n \tif {$tries_left > 0} {\n \t    after 100 [list config_check_tmp_exists $tries_left]\n \t} else {\n-\t    error_popup \"There appears to be a stale $config_file_tmp\\\n+\t    error_popup \"There appears to be a stale \\\n+ \\\"[file nativename $config_file_tmp]\\\" \\\n  file, which will prevent gitk from saving its configuration on exit.\\\n  Please remove it if it is not being used by any existing gitk process.\"\n \t}\n-- \n2.11.0.1122.gc3fec58.dirty\n\n"},{"id":"329091","messageId":"xmqq4lrn30bz.fsf@gitster.mtv.corp.google.com","threadId":"46851","inReplyTo":"20170928041417.28947-1-max@max630.net","subject":"Re: [PATCH] gitk: expand $config_file_tmp before reporting to user","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-09-28T04:37:36Z","receivedAt":"2017-09-28T04:37:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Max Kirillov <max@max630.net> writes:\n\n> Tilda-based path may confise some users. First, tilda is not known\n> for Window users, second, it may point to unexpected location\n> depending on various environment setup.\n>\n> Expand the path to \"nativename\", so that ~/.config/git/gitk-tmp\n> would be \"C:\\Users\\user\\.config\\git\\gitk-tmp\", for example.\n> It should be less cryptic\n\nIt might be less cryptic, but for those of us whose $HOME is a\nlooooooong path, ~/.config/git/gitk-tmp is much easier to understand\nthan the same path with ~/ expanded, which would push the part of\nthe filename that most matters far to the right hand side of the\ndialog.\n\nI somehow find this change just robbing Peter to pay Paul.\n\n>  \t} else {\n> -\t    error_popup \"There appears to be a stale $config_file_tmp\\\n> +\t    error_popup \"There appears to be a stale \\\n> + \\\"[file nativename $config_file_tmp]\\\" \\\n\n"},{"id":"329093","messageId":"xmqqzi9f1lb2.fsf@gitster.mtv.corp.google.com","threadId":"46851","inReplyTo":"xmqq4lrn30bz.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] gitk: expand $config_file_tmp before reporting to user","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-09-28T04:47:29Z","receivedAt":"2017-09-28T04:47:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Max Kirillov <max@max630.net> writes:\n>\n>> Tilda-based path may confise some users. First, tilda is not known\n>> for Window users, second, it may point to unexpected location\n>> depending on various environment setup.\n>>\n>> Expand the path to \"nativename\", so that ~/.config/git/gitk-tmp\n>> would be \"C:\\Users\\user\\.config\\git\\gitk-tmp\", for example.\n>> It should be less cryptic\n>\n> It might be less cryptic, but for those of us whose $HOME is a\n> looooooong path, ~/.config/git/gitk-tmp is much easier to understand\n> than the same path with ~/ expanded, which would push the part of\n> the filename that most matters far to the right hand side of the\n> dialog.\n>\n> I somehow find this change just robbing Peter to pay Paul.\n\nHaving said that, because a set-up might have HOME or XDG_CONFIG or\nother things misconfigured to point at a place where the end user\nmay not be expecting, I tend to think that catering to Paul by\nshowing the information closer to the bare metal is much more worthy\nthing to do than keeping Peter happy.  Since this is an error path,\naccuracy trumps convenience.\n\nSo no objection from me (unless somebody else comes up with an\nalternative that would make both camps happy, that is).\n\nThanks.\n\n"},{"id":"329105","messageId":"alpine.DEB.2.21.1.1709281428170.40514@virtualbox","threadId":"46851","inReplyTo":"xmqqzi9f1lb2.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] gitk: expand $config_file_tmp before reporting to user","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-09-28T12:31:17Z","receivedAt":"2017-09-28T12:31:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 28 Sep 2017, Junio C Hamano wrote:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> > Max Kirillov <max@max630.net> writes:\n> >\n> >> Tilda-based path may confise some users. First, tilda is not known\n> >> for Window users, second, it may point to unexpected location\n> >> depending on various environment setup.\n> >>\n> >> Expand the path to \"nativename\", so that ~/.config/git/gitk-tmp\n> >> would be \"C:\\Users\\user\\.config\\git\\gitk-tmp\", for example.\n> >> It should be less cryptic\n\nThanks, Max, for your contribution!\n\n> > It might be less cryptic, but for those of us whose $HOME is a\n> > looooooong path, ~/.config/git/gitk-tmp is much easier to understand\n> > than the same path with ~/ expanded, which would push the part of\n> > the filename that most matters far to the right hand side of the\n> > dialog.\n\nHeh, do you want to know how that must sound to a Windows user? I'm not\nsaying that I am a hard-core Windows user, but I do know a few, and I\nalready hear their comments in their own voice in my head...\n\n> > I somehow find this change just robbing Peter to pay Paul.\n> \n> Having said that, because a set-up might have HOME or XDG_CONFIG or\n> other things misconfigured to point at a place where the end user\n> may not be expecting, I tend to think that catering to Paul by\n> showing the information closer to the bare metal is much more worthy\n> thing to do than keeping Peter happy.  Since this is an error path,\n> accuracy trumps convenience.\n> \n> So no objection from me (unless somebody else comes up with an\n> alternative that would make both camps happy, that is).\n\nTo Unix/Linux users, the tilde (or Tilda, I really like that nickname, it\nmakes it much more human and tolerable) is probably *very* familiar.\n\nAs familiar, as it is unfamiliar to Windows users.\n\nSo I would actually suggest to make this a conditional on the platform: on\nWindows, use the native name, everywhere else, not.\n\nSound good?\nJohannes\n"},{"id":"329127","messageId":"xmqqzi9ezdk4.fsf@gitster.mtv.corp.google.com","threadId":"46851","inReplyTo":"alpine.DEB.2.21.1.1709281428170.40514@virtualbox","subject":"Re: [PATCH] gitk: expand $config_file_tmp before reporting to user","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-09-28T22:03:07Z","receivedAt":"2017-09-28T22:03:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> > Max Kirillov <max@max630.net> writes:\n>> >\n>> >> Tilda-based path may confise some users. First, tilda is not known\n>> >> for Window users, second, it may point to unexpected location\n>> >> depending on various environment setup.\n> ...\n> As familiar, as it is unfamiliar to Windows users.\n>\n> So I would actually suggest to make this a conditional on the platform: on\n> Windows, use the native name, everywhere else, not.\n>\n> Sound good?\n\nNot really.  \n\nI agree with and (more importantly) consider the second rationale\nMax cites a more relevant one for this change.  \n\nThis is about reporting an error, and using the short-hand ~/$rest\nthat could be pointing at a location different from what the user\n_thinks_ it points at for whatever reason (miconfigured HOME, some\nintermediate process setting it to something else, etc.) can hide\nthe real issue.  The problem can be more easily noticed and\ndiagnosed if the message shows the result of 'nativename'.  \n\nAnd that rationale holds whether you are seeing the error message on\nWindows or non-Windows.\n\n"},{"id":"329149","messageId":"20170929043406.GA9325@jessie.local","threadId":"46851","inReplyTo":"alpine.DEB.2.21.1.1709281428170.40514@virtualbox","subject":"Re: [PATCH] gitk: expand $config_file_tmp before reporting to user","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2017-09-29T04:34:06Z","receivedAt":"2017-09-29T04:34:22Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"On Thu, Sep 28, 2017 at 02:31:17PM +0200, Johannes Schindelin wrote:\n>>> Max Kirillov <max@max630.net> writes:\n>>>> Tilda-based path may confise some users. First, tilda is not known\n>>>> for Window users, second, it may point to unexpected location\n>>>> depending on various environment setup.\n>>>>\n>>>> Expand the path to \"nativename\", so that ~/.config/git/gitk-tmp\n>>>> would be \"C:\\Users\\user\\.config\\git\\gitk-tmp\", for example.\n>>>> It should be less cryptic\n\n> Thanks, Max, for your contribution!\n\nI do what I can. Just noticed s question at SO about it\n(https://stackoverflow.com/questions/46450479/how-to-remove-the-stale-gitk-tmp-file)\nProvided that I was author of the message, it looked like\nsomething for me to fix.\n\n> Sound good?\n\nAs Junio noticed, it would be more reliable to show full\npath, and error message does not have to be very nice\nanyway. Also, gitk is already too big and I always feel bad\nwhen adding stuff to it, so let's save couple of lines by\nnot adding another \"if\".\n\n-- \nMax\n"},{"id":"329212","messageId":"544DEAEA-9F66-471A-BEC9-B3D1F3C475AD@madiva.com","threadId":"46851","inReplyTo":"xmqq4lrn30bz.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] gitk: expand $config_file_tmp before reporting to user","fromName":"Uxío Prego","fromEmail":"uprego@madiva.com","sentAt":"2017-09-29T20:24:10Z","receivedAt":"2017-09-29T20:24:19Z","isPatch":true,"sender":{"key":"uprego@madiva.com","avatar":null},"body":"Well something like this is not paying _Paul_ enough for what he gave to The\nPeople, so I do not think there is worth trying.\n\n> On 28 Sep 2017, at 06:37, Junio C Hamano <gitster@pobox.com> wrote:\n> \n> Max Kirillov <max@max630.net> writes:\n> \n>> Tilda-based path may confise some users. First, tilda is not known\n>> for Window users, second, it may point to unexpected location\n>> depending on various environment setup.\n>> \n>> Expand the path to \"nativename\", so that ~/.config/git/gitk-tmp\n>> would be \"C:\\Users\\user\\.config\\git\\gitk-tmp\", for example.\n>> It should be less cryptic\n> \n> It might be less cryptic, but for those of us whose $HOME is a\n> looooooong path, ~/.config/git/gitk-tmp is much easier to understand\n> than the same path with ~/ expanded, which would push the part of\n> the filename that most matters far to the right hand side of the\n> dialog.\n> \n> I somehow find this change just robbing Peter to pay Paul.\n> \n>> \t} else {\n>> -\t    error_popup \"There appears to be a stale $config_file_tmp\\\n>> +\t    error_popup \"There appears to be a stale \\\n>> + \\\"[file nativename $config_file_tmp]\\\" \\\n> \n\n"}]}