{"thread":{"id":"51541","subject":"[PATCH] git-gui: Perform rescan on window focus-in","startedAt":"2019-07-28T15:17:37Z","lastAt":"2019-08-04T20:17:35Z","messageCount":23,"participants":["Pratyush Yadav","brian m. carlson","Mark Levedahl","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"379441","messageId":"20190728151726.9188-1-me@yadavpratyush.com","threadId":"51541","inReplyTo":null,"subject":"[PATCH] git-gui: Perform rescan on window focus-in","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-07-28T15:17:26Z","receivedAt":"2019-07-28T15:17:37Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"If any changes are made to the tree while git-gui is open, the user has\nto manually rescan to see those changes in the gui. With this change, a\nrescan will be performed whenever the window comes in focus, removing\nthe need for manual rescans in most cases. A manual rescan will still be\nneeded when something makes changes to the tree while git-gui is still\nin focus.\n\nSigned-off-by: Pratyush Yadav <me@yadavpratyush.com>\n---\n git-gui/git-gui.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\nindex 6de74ce639..8ca2033dc8 100755\n--- a/git-gui/git-gui.sh\n+++ b/git-gui/git-gui.sh\n@@ -3849,6 +3849,7 @@ if {[is_enabled transport]} {\n }\n \n bind .   <Key-F5>     ui_do_rescan\n+bind .   <FocusIn>    do_rescan\n bind .   <$M1B-Key-r> ui_do_rescan\n bind .   <$M1B-Key-R> ui_do_rescan\n bind .   <$M1B-Key-s> do_signoff\n-- \n2.22.0\n\n"},{"id":"379449","messageId":"20190728213634.GB162590@genre.crustytoothpaste.net","threadId":"51541","inReplyTo":"20190728151726.9188-1-me@yadavpratyush.com","subject":"Re: [PATCH] git-gui: Perform rescan on window focus-in","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-07-28T21:36:34Z","receivedAt":"2019-07-28T21:36:43Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2019-07-28 at 15:17:26, Pratyush Yadav wrote:\n> If any changes are made to the tree while git-gui is open, the user has\n> to manually rescan to see those changes in the gui. With this change, a\n> rescan will be performed whenever the window comes in focus, removing\n> the need for manual rescans in most cases. A manual rescan will still be\n> needed when something makes changes to the tree while git-gui is still\n> in focus.\n\nI don't use git-gui, so I have no opinion on this change either way, but\nI do have some questions.\n\nWhat exactly is involved in a rescan? Is it potentially expensive? If\nso, it might be better to leave it explicit, since people can\naccidentally give focus to the wrong window (bad click, focus follows\nmouse, etc.).\n\nIs there ever a situation in which someone might want to see the old\nstate? For example, would a rescan change the active commit shown?\nSomeone might be looking at a particular commit message or object ID for\nthe current commit; would this interfere with that?\n\n> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\n> index 6de74ce639..8ca2033dc8 100755\n> --- a/git-gui/git-gui.sh\n> +++ b/git-gui/git-gui.sh\n> @@ -3849,6 +3849,7 @@ if {[is_enabled transport]} {\n>  }\n>  \n>  bind .   <Key-F5>     ui_do_rescan\n> +bind .   <FocusIn>    do_rescan\n\nWhat's the difference between these two? Why are we using do_rescan\ninstead of ui_do_rescan?\n\n>  bind .   <$M1B-Key-r> ui_do_rescan\n>  bind .   <$M1B-Key-R> ui_do_rescan\n>  bind .   <$M1B-Key-s> do_signoff\n\nThe answers to a lot of these questions can go in your commit message to\nhelp reviewers and future readers understand your change better.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"379450","messageId":"c3cdbaba-ed57-ce6c-6c07-1e4fe8cc533f@yadavpratyush.com","threadId":"51541","inReplyTo":"20190728151726.9188-1-me@yadavpratyush.com","subject":"Re: [PATCH] git-gui: Perform rescan on window focus-in","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-07-28T21:44:42Z","receivedAt":"2019-07-28T21:44:50Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"Playing with it for some time, I see some problems with the patch.\n\nFirstly, \"bind .\" binds that event for the root window and all \nsubwindows. This means we rescan multiple times on a single FocusIn \nevent, once for every window. This can be fixed easily with a check for \nthe root window (aka \".\"). I'll send a v2 patch with this change.\n\nSecondly, when the context menu is opened via right click, and then \nclosed, it is recorded as a FocusIn event for our root window, even \nthough the context menu is a child of the root window. This triggers a \nrescan. Unfortunately, I could not find a way to go around this behaviour.\n\nThis means some unnecessary rescans, but the rescan looks like a pretty \nfast operation even on my slow hard disk, so it should probably not be a \nproblem.\n\nIf someone knows a way around the second issue, please let me know.\n\nOn 28/07/19 8:47 PM, Pratyush Yadav wrote:\n> If any changes are made to the tree while git-gui is open, the user has\n> to manually rescan to see those changes in the gui. With this change, a\n> rescan will be performed whenever the window comes in focus, removing\n> the need for manual rescans in most cases. A manual rescan will still be\n> needed when something makes changes to the tree while git-gui is still\n> in focus.\n> \n> Signed-off-by: Pratyush Yadav <me@yadavpratyush.com>\n> ---\n>   git-gui/git-gui.sh | 1 +\n>   1 file changed, 1 insertion(+)\n> \n> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\n> index 6de74ce639..8ca2033dc8 100755\n> --- a/git-gui/git-gui.sh\n> +++ b/git-gui/git-gui.sh\n> @@ -3849,6 +3849,7 @@ if {[is_enabled transport]} {\n>   }\n>   \n>   bind .   <Key-F5>     ui_do_rescan\n> +bind .   <FocusIn>    do_rescan\n>   bind .   <$M1B-Key-r> ui_do_rescan\n>   bind .   <$M1B-Key-R> ui_do_rescan\n>   bind .   <$M1B-Key-s> do_signoff\n> \n\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"379451","messageId":"e3f296a6-f33b-7b52-c4cb-9acf65145e64@yadavpratyush.com","threadId":"51541","inReplyTo":"20190728213634.GB162590@genre.crustytoothpaste.net","subject":"Re: [PATCH] git-gui: Perform rescan on window focus-in","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-07-28T22:10:29Z","receivedAt":"2019-07-28T22:10:42Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"Hi Brian,\n\nOn 29/07/19 3:06 AM, brian m. carlson wrote:\n> On 2019-07-28 at 15:17:26, Pratyush Yadav wrote:\n>> If any changes are made to the tree while git-gui is open, the user has\n>> to manually rescan to see those changes in the gui. With this change, a\n>> rescan will be performed whenever the window comes in focus, removing\n>> the need for manual rescans in most cases. A manual rescan will still be\n>> needed when something makes changes to the tree while git-gui is still\n>> in focus.\n> \n> I don't use git-gui, so I have no opinion on this change either way, but\n> I do have some questions.\n> \n> What exactly is involved in a rescan? Is it potentially expensive? If\n> so, it might be better to leave it explicit, since people can\n> accidentally give focus to the wrong window (bad click, focus follows\n> mouse, etc.).\n> \n\nThe function is not documented, and I only started spelunking the code a \ncouple days back, so I'll try to answer with what I know. It might not \nbe the full picture.\n\nRunning git-gui --trace, these commands are executed during a rescan:\n\n/usr/lib/git-core/git-rev-parse --verify HEAD\n/usr/lib/git-core/git-update-index -q --unmerged --ignore-missing --refresh\n\nSince I'm not too familiar with the details of these, I'll let you be \nthe judge on how expensive these operations are. But I'll add that \nrescans are pretty fast on my relatively slow hard disk.\n\n> Is there ever a situation in which someone might want to see the old\n> state? For example, would a rescan change the active commit shown?\n> Someone might be looking at a particular commit message or object ID for\n> the current commit; would this interfere with that?\n\nAt least in my workflow, there is no such situation. I use git-gui to \nlook at uncommitted changes, and stage and commit them. To look at older \ncommits, I use gitk. And from what I understand, git-gui is not designed \nto browse old commits. In the options menu, it simply opens gitk if you \nclick \"Visualise master's history\". There is no history browsing in \ngit-gui itself.\n\nYes, a rescan will remove the old commit and will show the latest \nchanges. This includes the old diff that was there before it was \ncommitted or removed from outside of git-gui. So this change will \ncertainly interfere with the old state.\n\n> \n>> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\n>> index 6de74ce639..8ca2033dc8 100755\n>> --- a/git-gui/git-gui.sh\n>> +++ b/git-gui/git-gui.sh\n>> @@ -3849,6 +3849,7 @@ if {[is_enabled transport]} {\n>>   }\n>>   \n>>   bind .   <Key-F5>     ui_do_rescan\n>> +bind .   <FocusIn>    do_rescan\n> \n> What's the difference between these two? Why are we using do_rescan\n> instead of ui_do_rescan?\n> \n\nui_do_rescan changes the focus to the first diff. It is executed when \nyou press F5 or choose Rescan from the menu. do_rescan does not do that.\n\nResetting to first diff on focus change will get annoying when you are \nin the middle of looking at some other file. do_rescan just updates the \nsoftware state without changing what file you are looking at or where in \nthat file you are looking at.\n\n>>   bind .   <$M1B-Key-r> ui_do_rescan\n>>   bind .   <$M1B-Key-R> ui_do_rescan\n>>   bind .   <$M1B-Key-s> do_signoff\n> \n> The answers to a lot of these questions can go in your commit message to\n> help reviewers and future readers understand your change better.\n> \n\nI'm never too sure what I should put in the commit message, so I took \nthe conservative route. I'll add more details in the v2 patch.\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"379453","messageId":"d5b2dfa0-2227-a982-0d6a-445dc1526813@yadavpratyush.com","threadId":"51541","inReplyTo":"e3f296a6-f33b-7b52-c4cb-9acf65145e64@yadavpratyush.com","subject":"Re: [PATCH] git-gui: Perform rescan on window focus-in","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-07-28T22:32:30Z","receivedAt":"2019-07-28T22:32:36Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 29/07/19 3:40 AM, Pratyush Yadav wrote:\n> Hi Brian,\n> \n> On 29/07/19 3:06 AM, brian m. carlson wrote:\n>> On 2019-07-28 at 15:17:26, Pratyush Yadav wrote:\n>>> If any changes are made to the tree while git-gui is open, the user has\n>>> to manually rescan to see those changes in the gui. With this change, a\n>>> rescan will be performed whenever the window comes in focus, removing\n>>> the need for manual rescans in most cases. A manual rescan will still be\n>>> needed when something makes changes to the tree while git-gui is still\n>>> in focus.\n>>\n>> I don't use git-gui, so I have no opinion on this change either way, but\n>> I do have some questions.\n>>\n>> What exactly is involved in a rescan? Is it potentially expensive? If\n>> so, it might be better to leave it explicit, since people can\n>> accidentally give focus to the wrong window (bad click, focus follows\n>> mouse, etc.).\n>>\n> \n> The function is not documented, and I only started spelunking the code a \n> couple days back, so I'll try to answer with what I know. It might not \n> be the full picture.\n> \n> Running git-gui --trace, these commands are executed during a rescan:\n> \n> /usr/lib/git-core/git-rev-parse --verify HEAD\n> /usr/lib/git-core/git-update-index -q --unmerged --ignore-missing --refresh\n> \n> Since I'm not too familiar with the details of these, I'll let you be \n> the judge on how expensive these operations are. But I'll add that \n> rescans are pretty fast on my relatively slow hard disk.\n> \n>> Is there ever a situation in which someone might want to see the old\n>> state? For example, would a rescan change the active commit shown?\n>> Someone might be looking at a particular commit message or object ID for\n>> the current commit; would this interfere with that?\n> \n> At least in my workflow, there is no such situation. I use git-gui to \n> look at uncommitted changes, and stage and commit them. To look at older \n> commits, I use gitk. And from what I understand, git-gui is not designed \n> to browse old commits. In the options menu, it simply opens gitk if you \n> click \"Visualise master's history\". There is no history browsing in \n> git-gui itself.\n> \n> Yes, a rescan will remove the old commit and will show the latest \n> changes. This includes the old diff that was there before it was \n> committed or removed from outside of git-gui. So this change will \n> certainly interfere with the old state.\n\nTo add to this, not rescanning has unwanted side effects too. Say you \nstage a file, and just as you are about to commit you suddenly remember \none small thing you forgot. So you add that change, and jump back into \ngit-gui. It shows your file as staged, and the unstaged changes section \nis empty. So you commit the change. Hit refresh and your last minute \nchange didn't make it in the commit, even though the unstaged changes \nlist was empty. This can cause some confusion, especially among newcomers.\n\n From my perspective, there is more to gain from auto rescanning than \nthere is to lose.\n\n> \n>>\n>>> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\n>>> index 6de74ce639..8ca2033dc8 100755\n>>> --- a/git-gui/git-gui.sh\n>>> +++ b/git-gui/git-gui.sh\n>>> @@ -3849,6 +3849,7 @@ if {[is_enabled transport]} {\n>>>   }\n>>>   bind .   <Key-F5>     ui_do_rescan\n>>> +bind .   <FocusIn>    do_rescan\n>>\n>> What's the difference between these two? Why are we using do_rescan\n>> instead of ui_do_rescan?\n>>\n> \n> ui_do_rescan changes the focus to the first diff. It is executed when \n> you press F5 or choose Rescan from the menu. do_rescan does not do that.\n> \n> Resetting to first diff on focus change will get annoying when you are \n> in the middle of looking at some other file. do_rescan just updates the \n> software state without changing what file you are looking at or where in \n> that file you are looking at.\n> \n>>>   bind .   <$M1B-Key-r> ui_do_rescan\n>>>   bind .   <$M1B-Key-R> ui_do_rescan\n>>>   bind .   <$M1B-Key-s> do_signoff\n>>\n>> The answers to a lot of these questions can go in your commit message to\n>> help reviewers and future readers understand your change better.\n>>\n> \n> I'm never too sure what I should put in the commit message, so I took \n> the conservative route. I'll add more details in the v2 patch.\n> \n\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"379454","messageId":"20190728224943.GC162590@genre.crustytoothpaste.net","threadId":"51541","inReplyTo":"e3f296a6-f33b-7b52-c4cb-9acf65145e64@yadavpratyush.com","subject":"Re: [PATCH] git-gui: Perform rescan on window focus-in","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-07-28T22:49:44Z","receivedAt":"2019-07-28T22:50:00Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2019-07-28 at 22:10:29, Pratyush Yadav wrote:\n> The function is not documented, and I only started spelunking the code a\n> couple days back, so I'll try to answer with what I know. It might not be\n> the full picture.\n> \n> Running git-gui --trace, these commands are executed during a rescan:\n> \n> /usr/lib/git-core/git-rev-parse --verify HEAD\n> /usr/lib/git-core/git-update-index -q --unmerged --ignore-missing --refresh\n> \n> Since I'm not too familiar with the details of these, I'll let you be the\n> judge on how expensive these operations are. But I'll add that rescans are\n> pretty fast on my relatively slow hard disk.\n\nThese are probably pretty cheap on all but the largest repositories. I\nwas worried we were enumerating all refs or all history or something\nlike that.\n\n> ui_do_rescan changes the focus to the first diff. It is executed when you\n> press F5 or choose Rescan from the menu. do_rescan does not do that.\n> \n> Resetting to first diff on focus change will get annoying when you are in\n> the middle of looking at some other file. do_rescan just updates the\n> software state without changing what file you are looking at or where in\n> that file you are looking at.\n\nYeah, this definitely seems like the right move.\n\n> I'm never too sure what I should put in the commit message, so I took the\n> conservative route. I'll add more details in the v2 patch.\n\nGreat. This sounds like a well-reasoned change. I'll let other folks who\nuse git-gui more chime in to see what they think as well.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"379461","messageId":"5027a6d9-7a6b-5f7d-759c-af967684b5b2@gmail.com","threadId":"51541","inReplyTo":"20190728224943.GC162590@genre.crustytoothpaste.net","subject":"Re: [PATCH] git-gui: Perform rescan on window focus-in","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2019-07-29T02:24:49Z","receivedAt":"2019-07-29T02:24:59Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"On 7/28/19 6:49 PM, brian m. carlson wrote:> On 2019-07-28 at 22:10:29, \nPratyush Yadav wrote:\n >> The function is not documented, and I only started spelunking the code a\n >> couple days back, so I'll try to answer with what I know. It might \nnot be\n >> the full picture.\n >>\n >> Running git-gui --trace, these commands are executed during a rescan:\n >>\n >> /usr/lib/git-core/git-rev-parse --verify HEAD\n >> /usr/lib/git-core/git-update-index -q --unmerged --ignore-missing \n--refresh\n >>\n >\n > Great. This sounds like a well-reasoned change. I'll let other folks who\n > use git-gui more chime in to see what they think as well.\n >\n\nI'm assuming this does what is currently done by F5.\nA simple rescan from git-gui in the git repository takes about 8 seconds \non my corporate laptop running Windows, making this happen on change of \nwindow focus is definitely not a friendly change from my view point.\n\nMark\n\n"},{"id":"379462","messageId":"724fb243-c660-ae04-1b2f-caf34794b799@gmail.com","threadId":"51541","inReplyTo":"20190728224943.GC162590@genre.crustytoothpaste.net","subject":"Re: [PATCH] git-gui: Perform rescan on window focus-in","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2019-07-29T02:28:11Z","receivedAt":"2019-07-29T02:28:16Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"On 7/28/19 6:49 PM, brian m. carlson wrote:> On 2019-07-28 at 22:10:29, \nPratyush Yadav wrote:\n >> The function is not documented, and I only started spelunking the code a\n >> couple days back, so I'll try to answer with what I know. It might \nnot be\n >> the full picture.\n >>\n >> Running git-gui --trace, these commands are executed during a rescan:\n >>\n >> /usr/lib/git-core/git-rev-parse --verify HEAD\n >> /usr/lib/git-core/git-update-index -q --unmerged --ignore-missing \n--refresh\n >>\n >\n > Great. This sounds like a well-reasoned change. I'll let other folks who\n > use git-gui more chime in to see what they think as well.\n >\n\nI'm assuming this does what is currently done by F5.\nA simple rescan from git-gui in the git repository takes about 8 seconds \non my corporate laptop running Windows, making this happen on change of \nwindow focus is definitely not a friendly change from my view point.\n\nMark\n"},{"id":"379463","messageId":"7082b89b-be6c-fd8f-fd8e-397b28b5b1c2@gmail.com","threadId":"51541","inReplyTo":"20190728224943.GC162590@genre.crustytoothpaste.net","subject":"Re: [PATCH] git-gui: Perform rescan on window focus-in","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2019-07-29T02:26:51Z","receivedAt":"2019-07-29T02:35:09Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"On 7/28/19 6:49 PM, brian m. carlson wrote:> On 2019-07-28 at 22:10:29, \nPratyush Yadav wrote:\n >> The function is not documented, and I only started spelunking the code a\n >> couple days back, so I'll try to answer with what I know. It might \nnot be\n >> the full picture.\n >>\n >> Running git-gui --trace, these commands are executed during a rescan:\n >>\n >> /usr/lib/git-core/git-rev-parse --verify HEAD\n >> /usr/lib/git-core/git-update-index -q --unmerged --ignore-missing \n--refresh\n >>\n >\n > Great. This sounds like a well-reasoned change. I'll let other folks who\n > use git-gui more chime in to see what they think as well.\n >\n\nI'm assuming this does what is currently done by F5.\nA simple rescan from git-gui in the git repository takes about 8 seconds \non my corporate laptop running Windows, making this happen on change of \nwindow focus is definitely not a friendly change from my view point.\n\nMark\n\n"},{"id":"379470","messageId":"xmqqef29l94s.fsf@gitster-ct.c.googlers.com","threadId":"51541","inReplyTo":"20190728224943.GC162590@genre.crustytoothpaste.net","subject":"Re: [PATCH] git-gui: Perform rescan on window focus-in","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-07-29T05:09:23Z","receivedAt":"2019-07-29T05:10:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> These are probably pretty cheap on all but the largest repositories. I\n> was worried we were enumerating all refs or all history or something\n> like that.\n>\n>> ui_do_rescan changes the focus to the first diff. It is executed when you\n>> press F5 or choose Rescan from the menu. do_rescan does not do that.\n>> \n>> Resetting to first diff on focus change will get annoying when you are in\n>> the middle of looking at some other file. do_rescan just updates the\n>> software state without changing what file you are looking at or where in\n>> that file you are looking at.\n>\n> Yeah, this definitely seems like the right move.\n\n\"Right move\" in the sense that it would try not to change what is\nbeing shown too much.  Rescan will still not be without cost, so it\nwill be annoying if it happens when the user did not make any\nchange.\n\nAnd it is annoying even more, if the user did make change in another\nwindow.  You may make a change perhaps from the command line, write\na short e-mail about it to let others know in your MUA, and then\nswitch the focus back to git-gui to continue working.  Refreshing\nupon git-gui getting focus is no better than manually pressing F5 or\nwhatever at that point.  It is too late at that point for spending\nextra cycles without being asked without getting annoying to the user.\n\nThe right time to spend cycles (in the background) in the above\nsample sequence is immediately after you made a change and switch to\nyour MUA---while you are typing a few paragraphs, you would not mind\ngit-gui spending seconds to repaint.\n\nSo probably a more productive use of our time, if we were to futz\nwith git-gui, would be to figure out how git-gui can have an\nbackground idle process that notices a change in the repository and\nrefreshes but only when you are *not* interacting with it (if it\ndoes things while you are interacting with it, it would become\nannoying and distracting), I would guess.  Just when you come back\nis the worst, and the most annoying, time to auto-refresh.\n\n"},{"id":"379476","messageId":"005d7946-3fbf-9c06-21fb-51f10d06f33e@yadavpratyush.com","threadId":"51541","inReplyTo":"724fb243-c660-ae04-1b2f-caf34794b799@gmail.com","subject":"Re: [PATCH] git-gui: Perform rescan on window focus-in","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-07-29T08:15:44Z","receivedAt":"2019-07-29T08:15:57Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 29/07/19 7:58 AM, Mark Levedahl wrote:\n> On 7/28/19 6:49 PM, brian m. carlson wrote:> On 2019-07-28 at 22:10:29, \n> Pratyush Yadav wrote:\n>  >> The function is not documented, and I only started spelunking the \n> code a\n>  >> couple days back, so I'll try to answer with what I know. It might \n> not be\n>  >> the full picture.\n>  >>\n>  >> Running git-gui --trace, these commands are executed during a rescan:\n>  >>\n>  >> /usr/lib/git-core/git-rev-parse --verify HEAD\n>  >> /usr/lib/git-core/git-update-index -q --unmerged --ignore-missing \n> --refresh\n>  >>\n>  >\n>  > Great. This sounds like a well-reasoned change. I'll let other folks who\n>  > use git-gui more chime in to see what they think as well.\n>  >\n> \n> I'm assuming this does what is currently done by F5.\n> A simple rescan from git-gui in the git repository takes about 8 seconds \n> on my corporate laptop running Windows, making this happen on change of \n> window focus is definitely not a friendly change from my view point.\n> \n\nThis is a Windows problem maybe? On my Linux machine with an almost dead \nhard drive, it takes under 10ms to do a refresh on the git repository \n(which has about 56,000 commits).\n\nCan you check what takes so long? I don't use Windows or I'd do this myself.\n\nGo to git-gui.sh and add some prints to the rescan procedure (line 1450) \nto trace which operation takes time. It is not a shell script, but a tcl \nscript, so you'd need to add \"puts <your_string_here>\" just to have an \napproximate idea of what takes so long.\n\nCan someone else who uses git-gui comment on the time taken to rescan on \nthe git repo (on Windows, Linux, or Mac)?\n\nYou can use this to see how long it takes to do a rescan. It will print \nthe time taken to stdout:\n\n-- >8 --\ndiff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\nindex 8ca2033dc8..7f2962f060 100755\n--- a/git-gui/git-gui.sh\n+++ b/git-gui/git-gui.sh\n@@ -2360,7 +2360,11 @@ proc do_rescan {} {\n  }\n\n  proc ui_do_rescan {} {\n+\tset t [clock milliseconds]\n  \trescan {force_first_diff ui_ready}\n+\tset t2 [clock milliseconds]\n+\n+\tputs \"Time elapsed in ui_do_rescan = [expr $t2 - $t]ms\"\n  }\n\n  proc do_commit {} {\n-- >8 --\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"379478","messageId":"beebae61-6858-b866-f388-7aa7cc5aa9b1@yadavpratyush.com","threadId":"51541","inReplyTo":"xmqqef29l94s.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] git-gui: Perform rescan on window focus-in","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-07-29T08:44:29Z","receivedAt":"2019-07-29T08:44:36Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 29/07/19 10:39 AM, Junio C Hamano wrote:\n> \"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n> \n>> These are probably pretty cheap on all but the largest repositories. I\n>> was worried we were enumerating all refs or all history or something\n>> like that.\n>>\n>>> ui_do_rescan changes the focus to the first diff. It is executed when you\n>>> press F5 or choose Rescan from the menu. do_rescan does not do that.\n>>>\n>>> Resetting to first diff on focus change will get annoying when you are in\n>>> the middle of looking at some other file. do_rescan just updates the\n>>> software state without changing what file you are looking at or where in\n>>> that file you are looking at.\n>>\n>> Yeah, this definitely seems like the right move.\n> \n> \"Right move\" in the sense that it would try not to change what is\n> being shown too much.  Rescan will still not be without cost, so it\n> will be annoying if it happens when the user did not make any\n> change.\n> \n> And it is annoying even more, if the user did make change in another\n> window.  You may make a change perhaps from the command line, write\n> a short e-mail about it to let others know in your MUA, and then\n> switch the focus back to git-gui to continue working.  Refreshing\n> upon git-gui getting focus is no better than manually pressing F5 or\n> whatever at that point.  It is too late at that point for spending\n> extra cycles without being asked without getting annoying to the user.\n\nOn my system with a not-so-fast hard disk, it takes under 10ms to do a \nrescan (4 to 7ms usually). Using it for some time, I never felt any \nlatency or lag. Maybe it will cause problems on other systems, but I'd \nlike to hear from more people about it.\n\nAlso, why is it no better than manually refreshing? Manually refreshing \ngets annoying real quick (and that's why I sent this change in the first \nplace). Not having to hit refresh every time you make any change is \npretty sweet.\n\nYes it maybe doesn't matter as much when you open git-gui _after_ you're \ndone making changes, and now only have to write the commit message. But \nif you keep it open while you are editing code, and then when you are \ndone, you switch to it to commit, auto refresh is a great quality of \nlife improvement.\n\nAgain, on my system at least, there is no noticeable latency when \nswitching focus. How about making rescan on focus optional? The user can \ndisable it if they feel like it is getting in their way.\n\n> \n> The right time to spend cycles (in the background) in the above\n> sample sequence is immediately after you made a change and switch to\n> your MUA---while you are typing a few paragraphs, you would not mind\n> git-gui spending seconds to repaint.\n\nFYI, we are not just repainting, we are also syncing the repo state with \nthe in-memory state when doing a rescan.\n\n> \n> So probably a more productive use of our time, if we were to futz\n> with git-gui, would be to figure out how git-gui can have an\n> background idle process that notices a change in the repository and\n> refreshes but only when you are *not* interacting with it (if it\n> does things while you are interacting with it, it would become\n> annoying and distracting), I would guess.  Just when you come back\n> is the worst, and the most annoying, time to auto-refresh.\n> \n\nSo maybe something like inotify? tcl-inotify [0] (a tcl extension) can \nbe used for it I suppose, though I haven't looked into it much yet.\n\nMy idea with this change was to keep it as minimal as possible. Doing a \nrescan on focus seems like a good compromise to me. I can look into \ninotify, but I'm not too aware of the performance impact it can have.\n\nAlso, do you know how editors like Atom or VSCode refresh when there is \na change to the tree? I've seen them auto-update the GUI when you make \nany changes from outside of them.\n\nI will look into tcl-inotify if there is a consensus that it is the \nbetter way to go about this. It seems like a significant time \ninvestment, and I'd rather not spend a lot of time doing something that \nwould get rejected instantly.\n\n[0] http://tcl-inotify.sourceforge.net/\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"379704","messageId":"nycvar.QRO.7.76.6.1907312132190.21907@tvgsbejvaqbjf.bet","threadId":"51541","inReplyTo":"005d7946-3fbf-9c06-21fb-51f10d06f33e@yadavpratyush.com","subject":"Re: [PATCH] git-gui: Perform rescan on window focus-in","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-07-31T19:42:07Z","receivedAt":"2019-07-31T19:42:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 29 Jul 2019, Pratyush Yadav wrote:\n\n> On 29/07/19 7:58 AM, Mark Levedahl wrote:\n> > On 7/28/19 6:49 PM, brian m. carlson wrote:> On 2019-07-28 at\n> > 22:10:29, Pratyush Yadav wrote:\n> > > > The function is not documented, and I only started spelunking\n> > > > the code a couple days back, so I'll try to answer with what I\n> > > > know. It might not be the full picture.\n> > > >\n> > > > Running git-gui --trace, these commands are executed during a rescan:\n> > > >\n> > > > /usr/lib/git-core/git-rev-parse --verify HEAD\n> > > > /usr/lib/git-core/git-update-index -q --unmerged --ignore-missing --refresh\n> > > >\n> > >\n> > > Great. This sounds like a well-reasoned change. I'll let other folks who\n> > > use git-gui more chime in to see what they think as well.\n> > >\n> >\n> > I'm assuming this does what is currently done by F5.\n> > A simple rescan from git-gui in the git repository takes about 8 seconds on\n> > my corporate laptop running Windows, making this happen on change of window\n> > focus is definitely not a friendly change from my view point.\n> >\n>\n> This is a Windows problem maybe? On my Linux machine with an almost dead hard\n> drive, it takes under 10ms to do a refresh on the git repository (which has\n> about 56,000 commits).\n\nI would be _extremely_ cautious to base an argument on one particular\nsetup, using on particular hardware with one particular OS and one\nparticular repository.\n\nWhen it comes to repositories that are worked on actively, git.git is\nnot actually a representative example, it is way smaller than what users\ndeal with.\n\nYou might be one of those developers privileged enough to have a fast\ncomputer. Trying to extrapolate from such a vantage point does the rest\nof us Git users a big disservice.\n\nAt this point, I am gently inclined against the presented approach, in\nparticular given that even context menus reportedly trigger the re-scan\n(which I suspect might actually be a Linux-only issue, as context menus\nare top-level windows on X11, at least if I remember correctly, and I\nalso seem to remember that they are dependent windows on Aqua and Win32,\njust to add yet another argument against overfitting considerations onto\na single, specific setup).\n\nCiao,\nJohannes\n"},{"id":"379786","messageId":"cc5dddc7-e33e-2a2c-3205-6dd14edd0abd@yadavpratyush.com","threadId":"51541","inReplyTo":"nycvar.QRO.7.76.6.1907312132190.21907@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] git-gui: Perform rescan on window focus-in","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-08-01T21:52:09Z","receivedAt":"2019-08-01T21:52:16Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"+Junio\n\nOn 8/1/19 1:12 AM, Johannes Schindelin wrote:\n> Hi,\n> \n> On Mon, 29 Jul 2019, Pratyush Yadav wrote:\n> \n>> On 29/07/19 7:58 AM, Mark Levedahl wrote:\n>>> On 7/28/19 6:49 PM, brian m. carlson wrote:> On 2019-07-28 at\n>>> 22:10:29, Pratyush Yadav wrote:\n>>>>> The function is not documented, and I only started spelunking\n>>>>> the code a couple days back, so I'll try to answer with what I\n>>>>> know. It might not be the full picture.\n>>>>>\n>>>>> Running git-gui --trace, these commands are executed during a rescan:\n>>>>>\n>>>>> /usr/lib/git-core/git-rev-parse --verify HEAD\n>>>>> /usr/lib/git-core/git-update-index -q --unmerged --ignore-missing --refresh\n>>>>>\n>>>>\n>>>> Great. This sounds like a well-reasoned change. I'll let other folks who\n>>>> use git-gui more chime in to see what they think as well.\n>>>>\n>>>\n>>> I'm assuming this does what is currently done by F5.\n>>> A simple rescan from git-gui in the git repository takes about 8 seconds on\n>>> my corporate laptop running Windows, making this happen on change of window\n>>> focus is definitely not a friendly change from my view point.\n>>>\n>>\n>> This is a Windows problem maybe? On my Linux machine with an almost dead hard\n>> drive, it takes under 10ms to do a refresh on the git repository (which has\n>> about 56,000 commits).\n> \n> I would be _extremely_ cautious to base an argument on one particular\n> setup, using on particular hardware with one particular OS and one\n> particular repository.\n> \n\nAgreed. That's why I asked for benchmarks from other people. \nUnfortunately, no one replied.\n\nI am worried about exactly this problem that other users will have \nperformance problems. I usually reserve judgment till I see some actual \nbenchmarks, but since in this case we aren't getting any, it is probably \nbetter to err on the side of caution.\n\n> When it comes to repositories that are worked on actively, git.git is\n> not actually a representative example, it is way smaller than what users\n> deal with.\n\nOut of curiosity, what would you consider large enough? The Linux kernel \n(855,753 commits as of writing this)?\n\n> \n> You might be one of those developers privileged enough to have a fast\n> computer. Trying to extrapolate from such a vantage point does the rest\n> of us Git users a big disservice.\n\nYes I have a pretty powerful machine in the processor department, but I \nhave a rather slow hard disk. I assumed most people would have faster \nhard disks than me, but in hindsight, that might not be a good \nassumption to make.\n\n> \n> At this point, I am gently inclined against the presented approach, in\n> particular given that even context menus reportedly trigger the re-scan\n> (which I suspect might actually be a Linux-only issue, as context menus\n> are top-level windows on X11, at least if I remember correctly, and I\n> also seem to remember that they are dependent windows on Aqua and Win32,\n> just to add yet another argument against overfitting considerations onto\n> a single, specific setup).\n\nAll right, the patch in its current state can't fly. So what is the \ncorrect way to do this? I see the following options:\n\n1. Add this as an option that is disabled by default, but people who \ndon't mind it can enable it. This is the easiest to implement. But I \nleave it to you and Junio (and anyone else who wants to pitch in :)) to \ndecide if it is a good idea.\n\n2. Watch all files for changes. Atom does this for their git gui [0]. We \ncan probably use watchman [1] for this, but this adds another external \ndependency.\n\n3. Leave this feature out. I of course don't like this option very much, \nand will probably have to run a fork, but if it is better for the \nproject, it is better for the project.\n\nWhich option do you want to go with?\n\nNaturally, my favourite option is number 1 because it will be the \neasiest to implement ;)\n\n[0] \nhttps://discuss.atom.io/t/how-does-atom-refresh-git-state-when-some-external-editor-changes-it/67210\n[1] https://github.com/facebook/watchman\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"379823","messageId":"nycvar.QRO.7.76.6.1908021414530.46@tvgsbejvaqbjf.bet","threadId":"51541","inReplyTo":"cc5dddc7-e33e-2a2c-3205-6dd14edd0abd@yadavpratyush.com","subject":"Re: [PATCH] git-gui: Perform rescan on window focus-in","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-08-02T12:39:17Z","receivedAt":"2019-08-02T12:39:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 2 Aug 2019, Pratyush Yadav wrote:\n\n> On 8/1/19 1:12 AM, Johannes Schindelin wrote:\n> >\n> > I would be _extremely_ cautious to base an argument on one\n> > particular setup, using on particular hardware with one particular\n> > OS and one particular repository.\n> >\n>\n> Agreed. That's why I asked for benchmarks from other people.\n> Unfortunately, no one replied.\n\nWhat stops _you_ from performing more tests yourself? There are tons of\nreal-world repositories out there, we even talk about options for large\nrepositories to test with in Git for Windows' Contributing Guidelines:\nhttps://github.com/git-for-windows/git/blob/master/CONTRIBUTING.md#performance-tests\n\n> I am worried about exactly this problem that other users will have\n> performance problems. I usually reserve judgment till I see some\n> actual benchmarks, but since in this case we aren't getting any, it is\n> probably better to err on the side of caution.\n\nWell, you proposed it without testing it first e.g. on Kotlin or even\nLinux.  Personally, I would have chosen Chromium, as it is a pretty big\none.\n\nMaybe you want to do that now.\n\n> > When it comes to repositories that are worked on actively, git.git is\n> > not actually a representative example, it is way smaller than what users\n> > deal with.\n>\n> Out of curiosity, what would you consider large enough? The Linux kernel\n> (855,753 commits as of writing this)?\n\nAs your patch is about refreshing the Git GUI, the number of Git commits\nis irrelevant. The number of worktree files is relevant.\n\nUnfortunately, you cannot search for this dimension on GitHub, but you\ncan search for \"repository size\", which is at least loosely correlated:\n\nhttps://github.com/search?utf8=%E2%9C%93&q=size%3A%3E5000000&type=Repositories&ref=advsearch&l=&l=\n\nPersonally, I would probably have chosen either of these, for testing:\n\n* https://github.com/chromium/chromium\n* https://github.com/WebKit/webkit\n* https://github.com/raspberrypi/firmware\n* https://github.com/MicrosoftDocs/azure-docs\n* https://github.com/facebookresearch/FAIR-Play\n\n> > At this point, I am gently inclined against the presented approach, in\n> > particular given that even context menus reportedly trigger the re-scan\n> > (which I suspect might actually be a Linux-only issue, as context menus\n> > are top-level windows on X11, at least if I remember correctly, and I\n> > also seem to remember that they are dependent windows on Aqua and Win32,\n> > just to add yet another argument against overfitting considerations onto\n> > a single, specific setup).\n>\n> All right, the patch in its current state can't fly. So what is the correct\n> way to do this? I see the following options:\n>\n> 1. Add this as an option that is disabled by default, but people who don't\n> mind it can enable it. This is the easiest to implement. But I leave it to you\n> and Junio (and anyone else who wants to pitch in :)) to decide if it is a good\n> idea.\n\nThat would probably be much easier to get accepted.\n\n> 2. Watch all files for changes. Atom does this for their git gui [0]. We can\n> probably use watchman [1] for this, but this adds another external dependency.\n\nI am currently looking at watchman, and it seems that it has its own\nperformance issues in big repositories (for which it is actually most\nrelevant). Besides, Windows support is kinda flaky, so I would rule this\nout (Git is supported on many more platforms than watchman supports).\n\nBesides, what your patch wants to do is not to know when things have\nchanged. Your patch wants to refresh the UI at opportune moments, and it\nis unclear how watchman could help decide when to refresh.\n\n> 3. Leave this feature out. I of course don't like this option very much, and\n> will probably have to run a fork, but if it is better for the project, it is\n> better for the project.\n\nThat would indeed be the safest.\n\nI wonder, however, whether you can think of a better method to figure\nout when to auto-refresh. Focus seems to be a low-hanging fruit, but as\nyou noticed it is not very accurate. Maybe if you combine it with a\ntimeout? Or maybe you can detect idle time in Tcl/Tk?\n\nCiao,\nJohannes\n"},{"id":"379839","messageId":"xmqqftmjldjx.fsf@gitster-ct.c.googlers.com","threadId":"51541","inReplyTo":"cc5dddc7-e33e-2a2c-3205-6dd14edd0abd@yadavpratyush.com","subject":"Re: [PATCH] git-gui: Perform rescan on window focus-in","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-08-02T16:47:30Z","receivedAt":"2019-08-02T16:47:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pratyush Yadav <me@yadavpratyush.com> writes:\n\n> +Junio\n\nI do not have a strong opinion on this one---a Meh by default means\na moderately strong preference for status-quo.\n\n> All right, the patch in its current state can't fly. So what is the\n> correct way to do this? I see the following options:\n>\n> 1. Add this as an option that is disabled by default, but people who\n> don't mind it can enable it. This is the easiest to implement. But I\n> leave it to you and Junio (and anyone else who wants to pitch in :))\n> to decide if it is a good idea.\n\nI think this is a good first step.  As I said already, I am not\nconvinced that \"focus in\" is a good heuristics for triggering auto\nrescan, and I suspect that you or others may come up with and\nreplace it with a better heuristic over time.  During that\nexperiment, it would be better to allow interested others to opt\ninto the feature to help, while not disturbing ordinary users who\nare OK with the current behaviour.\n"},{"id":"379851","messageId":"a68f09be-949f-b16b-a585-9ca2a1991a4f@yadavpratyush.com","threadId":"51541","inReplyTo":"nycvar.QRO.7.76.6.1908021414530.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] git-gui: Perform rescan on window focus-in","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-08-02T20:00:13Z","receivedAt":"2019-08-02T20:00:20Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 8/2/19 6:09 PM, Johannes Schindelin wrote:\n> Hi,\n> \n> On Fri, 2 Aug 2019, Pratyush Yadav wrote:\n> \n>> On 8/1/19 1:12 AM, Johannes Schindelin wrote:\n>>>\n>>> I would be _extremely_ cautious to base an argument on one\n>>> particular setup, using on particular hardware with one particular\n>>> OS and one particular repository.\n>>>\n>>\n>> Agreed. That's why I asked for benchmarks from other people.\n>> Unfortunately, no one replied.\n> \n> What stops _you_ from performing more tests yourself? There are tons of\n> real-world repositories out there, we even talk about options for large\n> repositories to test with in Git for Windows' Contributing Guidelines:\n> https://github.com/git-for-windows/git/blob/master/CONTRIBUTING.md#performance-tests\n\nI thought the point was to not base all data off a single setup?\n\nAnyway, I'll see if I can manage to clone the Chromium source (I have \nflaky internet), and I'll run some tests.\n\nAside: I looked into the resumeable clones discussion in the mailing \nlist archives. I don't think the feature made it through. But the work \nseems too complicated for me to look into reviving it. Maybe some other day.\n\n> \n>> I am worried about exactly this problem that other users will have\n>> performance problems. I usually reserve judgment till I see some\n>> actual benchmarks, but since in this case we aren't getting any, it is\n>> probably better to err on the side of caution.\n> \n> Well, you proposed it without testing it first e.g. on Kotlin or even\n> Linux.  Personally, I would have chosen Chromium, as it is a pretty big\n> one.\n\nWhen I first proposed the patch, I thought the git repo was large \nenough. And testing on it was pretty fast for me.\n\n> \n> Maybe you want to do that now.\n> \n>>> When it comes to repositories that are worked on actively, git.git is\n>>> not actually a representative example, it is way smaller than what users\n>>> deal with.\n>>\n>> Out of curiosity, what would you consider large enough? The Linux kernel\n>> (855,753 commits as of writing this)?\n> \n> As your patch is about refreshing the Git GUI, the number of Git commits\n> is irrelevant. The number of worktree files is relevant.\n> \n> Unfortunately, you cannot search for this dimension on GitHub, but you\n> can search for \"repository size\", which is at least loosely correlated:\n> \n> https://github.com/search?utf8=%E2%9C%93&q=size%3A%3E5000000&type=Repositories&ref=advsearch&l=&l=\n> \n> Personally, I would probably have chosen either of these, for testing:\n> \n> * https://github.com/chromium/chromium\n> * https://github.com/WebKit/webkit\n> * https://github.com/raspberrypi/firmware\n> * https://github.com/MicrosoftDocs/azure-docs\n> * https://github.com/facebookresearch/FAIR-Play\n> \n>>> At this point, I am gently inclined against the presented approach, in\n>>> particular given that even context menus reportedly trigger the re-scan\n>>> (which I suspect might actually be a Linux-only issue, as context menus\n>>> are top-level windows on X11, at least if I remember correctly, and I\n>>> also seem to remember that they are dependent windows on Aqua and Win32,\n>>> just to add yet another argument against overfitting considerations onto\n>>> a single, specific setup).\n>>\n>> All right, the patch in its current state can't fly. So what is the correct\n>> way to do this? I see the following options:\n>>\n>> 1. Add this as an option that is disabled by default, but people who don't\n>> mind it can enable it. This is the easiest to implement. But I leave it to you\n>> and Junio (and anyone else who wants to pitch in :)) to decide if it is a good\n>> idea.\n> \n> That would probably be much easier to get accepted.\n> \n>> 2. Watch all files for changes. Atom does this for their git gui [0]. We can\n>> probably use watchman [1] for this, but this adds another external dependency.\n> \n> I am currently looking at watchman, and it seems that it has its own\n> performance issues in big repositories (for which it is actually most\n> relevant). Besides, Windows support is kinda flaky, so I would rule this\n> out (Git is supported on many more platforms than watchman supports).\n> \n> Besides, what your patch wants to do is not to know when things have\n> changed. Your patch wants to refresh the UI at opportune moments, and it\n> is unclear how watchman could help decide when to refresh.\n>\n\nBut we do want to know when things have changed, because that's when we \nare supposed to refresh. The most opportune moment to refresh is right \nwhen some file changes. The user is very likely to not be looking at git \ngui at that point because they'd probably in their editor, so we get to \nrefresh in the background. Watchman lets us do this, but it is just an \nexample. Any cross-platform method of watching a directory tree works \njust as well.\n\nMaybe we can use Watchdog [0], which might be better (disclaimer: I \nhaven't actually looked into watchdog, I'm just saying it maybe is \nbetter). It is an added dependency regardless.\n\nA Linux-only alternative is tcl-inotify [1], but I don't know if you \nfolks will like a platform dependent solution.\n\n>> 3. Leave this feature out. I of course don't like this option very much, and\n>> will probably have to run a fork, but if it is better for the project, it is\n>> better for the project.\n> \n> That would indeed be the safest.\n> \n> I wonder, however, whether you can think of a better method to figure\n> out when to auto-refresh. Focus seems to be a low-hanging fruit, but as\n> you noticed it is not very accurate. Maybe if you combine it with a\n> timeout? Or maybe you can detect idle time in Tcl/Tk?\n> \n\nHm, I don't see a better alternative than file system watches. Timeouts \nare a heuristic that can potentially be problematic. If you do a refresh \ntoo frequently, you hog up the user's resources for little benefit. You \ndon't refresh frequently enough, then sometimes the user sees an updated \nview, sometimes a not-updated view. This inconsistency, if not fine \ntuned properly, can prove detrimental to UX.\n\nIf you are fine with the above inconsistency, refreshing every few \nseconds shouldn't be too difficult.\n\n[0] https://pythonhosted.org/watchdog/\n[1] http://tcl-inotify.sourceforge.net/\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"379852","messageId":"a143dcfd-8067-34f7-97f3-c4482dca5819@yadavpratyush.com","threadId":"51541","inReplyTo":"xmqqftmjldjx.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] git-gui: Perform rescan on window focus-in","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-08-02T20:13:11Z","receivedAt":"2019-08-02T20:13:19Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 8/2/19 10:17 PM, Junio C Hamano wrote:\n> Pratyush Yadav <me@yadavpratyush.com> writes:\n> \n>> +Junio\n> \n> I do not have a strong opinion on this one---a Meh by default means\n> a moderately strong preference for status-quo.\n> \n>> All right, the patch in its current state can't fly. So what is the\n>> correct way to do this? I see the following options:\n>>\n>> 1. Add this as an option that is disabled by default, but people who\n>> don't mind it can enable it. This is the easiest to implement. But I\n>> leave it to you and Junio (and anyone else who wants to pitch in :))\n>> to decide if it is a good idea.\n> \n> I think this is a good first step.  As I said already, I am not\n> convinced that \"focus in\" is a good heuristics for triggering auto\n> rescan, and I suspect that you or others may come up with and\n> replace it with a better heuristic over time.  During that\n> experiment, it would be better to allow interested others to opt\n> into the feature to help, while not disturbing ordinary users who\n> are OK with the current behaviour.\n> \n\nAll right. I'll look a bit more to see if I can figure something better. \nI suggested using filesystem watches. I'll wait a bit to hear from \nJohannes on what he thinks about it. He doesn't like the idea of using \nWatchman apparently.\n\nOtherwise, I'll send in an updated patch with this feature as opt-in.\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"379887","messageId":"nycvar.QRO.7.76.6.1908032225510.46@tvgsbejvaqbjf.bet","threadId":"51541","inReplyTo":"a68f09be-949f-b16b-a585-9ca2a1991a4f@yadavpratyush.com","subject":"Re: [PATCH] git-gui: Perform rescan on window focus-in","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-08-03T20:34:42Z","receivedAt":"2019-08-03T20:44:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 3 Aug 2019, Pratyush Yadav wrote:\n\n> On 8/2/19 6:09 PM, Johannes Schindelin wrote:\n> >\n> > On Fri, 2 Aug 2019, Pratyush Yadav wrote:\n> >\n> > > On 8/1/19 1:12 AM, Johannes Schindelin wrote:\n> > > >\n> > > > I would be _extremely_ cautious to base an argument on one\n> > > > particular setup, using on particular hardware with one\n> > > > particular OS and one particular repository.\n> > > >\n> > >\n> > > Agreed. That's why I asked for benchmarks from other people.\n> > > Unfortunately, no one replied.\n> >\n> > What stops _you_ from performing more tests yourself? There are tons\n> > of real-world repositories out there, we even talk about options for\n> > large repositories to test with in Git for Windows' Contributing\n> > Guidelines:\n> > https://github.com/git-for-windows/git/blob/master/CONTRIBUTING.md#performance-tests\n>\n> I thought the point was to not base all data off a single setup?\n\nYou misunderstood what I was saying: a single setup is bad, and you can\nmake it moderately better by testing _at least_ with a moderately-sized\nrepository [*1*] in addition to git.git.\n\nSo yes, it would still not be enough to test with, say, the git.git\n_and_ the Chromium repository _only_ on your setup, but if not even you\ncan be bothered to test with more than one small repository, how can you\npossibly expect anybody else to add more testing?\n\n> [...]\n> > I wonder, however, whether you can think of a better method to\n> > figure out when to auto-refresh. Focus seems to be a low-hanging\n> > fruit, but as you noticed it is not very accurate. Maybe if you\n> > combine it with a timeout? Or maybe you can detect idle time in\n> > Tcl/Tk?\n>\n> Hm, I don't see a better alternative than file system watches.\n> Timeouts are a heuristic that can potentially be problematic.\n\nLet me stress the fact that I suggested a timeout _in addition_ to the\nfocus event?\n\nYes, using a timeout on its own is stupidly problematic. That's why I\ndid not suggest that.\n\n> If you do a refresh too frequently, you hog up the user's resources\n> for little benefit.\n\nIndeed. You want to find a heuristic that catches most of the cases\nwhere files were changed, while at the same time not even _trying_ to\nrefresh automatically when probably nothing changed.\n\n> You don't refresh frequently enough, then sometimes the user sees an\n> updated view, sometimes a not-updated view. This inconsistency, if not\n> fine tuned properly, can prove detrimental to UX.\n\nRight, exactly. And of course, since you are not changing your own Git\nfork only, but the Git that is the one that most users will use, you\nwill have to consider carefully in which direction to strike the balance\nby default.\n\nI would contend that you should err on the side of _not_ refreshing, as\nthat would probably affect more users negatively than the opposite.\n\nCiao,\nJohannes\n\nFootnote *1*: I don't expect you to test with the largest repositories,\nas you are unlikely to have access to anything approaching the size of\nthe largest Git repository on the planet:\nhttps://devblogs.microsoft.com/bharry/the-largest-git-repo-on-the-planet/\n"},{"id":"379912","messageId":"1489be05-ab18-a3e5-dd38-3d5729ebe67a@yadavpratyush.com","threadId":"51541","inReplyTo":"nycvar.QRO.7.76.6.1908032225510.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] git-gui: Perform rescan on window focus-in","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-08-04T12:53:15Z","receivedAt":"2019-08-04T12:53:28Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 8/4/19 2:04 AM, Johannes Schindelin wrote:\n> Hi,\n> \n> On Sat, 3 Aug 2019, Pratyush Yadav wrote:\n> \n>> On 8/2/19 6:09 PM, Johannes Schindelin wrote:\n>>>\n>>> On Fri, 2 Aug 2019, Pratyush Yadav wrote:\n>>>\n>>>> On 8/1/19 1:12 AM, Johannes Schindelin wrote:\n>>>>>\n>>>>> I would be _extremely_ cautious to base an argument on one\n>>>>> particular setup, using on particular hardware with one\n>>>>> particular OS and one particular repository.\n>>>>>\n>>>>\n>>>> Agreed. That's why I asked for benchmarks from other people.\n>>>> Unfortunately, no one replied.\n>>>\n>>> What stops _you_ from performing more tests yourself? There are tons\n>>> of real-world repositories out there, we even talk about options for\n>>> large repositories to test with in Git for Windows' Contributing\n>>> Guidelines:\n>>> https://github.com/git-for-windows/git/blob/master/CONTRIBUTING.md#performance-tests\n>>\n>> I thought the point was to not base all data off a single setup?\n> \n> You misunderstood what I was saying: a single setup is bad, and you can\n> make it moderately better by testing _at least_ with a moderately-sized\n> repository [*1*] in addition to git.git.\n> \n> So yes, it would still not be enough to test with, say, the git.git\n> _and_ the Chromium repository _only_ on your setup, but if not even you\n> can be bothered to test with more than one small repository, how can you\n> possibly expect anybody else to add more testing?\n\nAll right, I'll see what repos I can test.\n\nBut my internet is pretty slow and unstable, so my clone of the Chromium \nrepo failed mid-way multiple times. I assume we need to test on a large \nindex, so is it all right if I use t/perf/repos/many-files.sh to \nartificially generate a large repo?\n\n> \n>> [...]\n>>> I wonder, however, whether you can think of a better method to\n>>> figure out when to auto-refresh. Focus seems to be a low-hanging\n>>> fruit, but as you noticed it is not very accurate. Maybe if you\n>>> combine it with a timeout? Or maybe you can detect idle time in\n>>> Tcl/Tk?\n>>\n>> Hm, I don't see a better alternative than file system watches.\n>> Timeouts are a heuristic that can potentially be problematic.\n> \n> Let me stress the fact that I suggested a timeout _in addition_ to the\n> focus event?\n\nOh, my bad. I thought you suggested using timeouts exclusively.\n\nBut I'm not sure I understand what you mean by \"using timeouts in \naddition to the focus event\". My guess is that you mean we should \nactivate a refresh-on-focus-in only after git-gui has been out of focus \nfor a certain amount of time. Is my guess correct?\n\n> \n> Yes, using a timeout on its own is stupidly problematic. That's why I\n> did not suggest that.\n> \n>> If you do a refresh too frequently, you hog up the user's resources\n>> for little benefit.\n> \n> Indeed. You want to find a heuristic that catches most of the cases\n> where files were changed, while at the same time not even _trying_ to\n> refresh automatically when probably nothing changed.\n\nLike I said before, the best way of doing that that I can see is file \nsystem watches. But maybe we can get reasonable performance with a \ncombination of timeouts and focus events.\n\n> \n>> You don't refresh frequently enough, then sometimes the user sees an\n>> updated view, sometimes a not-updated view. This inconsistency, if not\n>> fine tuned properly, can prove detrimental to UX.\n> \n> Right, exactly. And of course, since you are not changing your own Git\n> fork only, but the Git that is the one that most users will use, you\n> will have to consider carefully in which direction to strike the balance\n> by default.\n\nYep, that's why I'm engaging you in this conversation :). I'm personally \npretty happy with this change for my own use, but I'd like everyone \nenjoy it equally.\n\n> \n> I would contend that you should err on the side of _not_ refreshing, as\n> that would probably affect more users negatively than the opposite.\n> \n> Ciao,\n> Johannes\n> \n> Footnote *1*: I don't expect you to test with the largest repositories,\n> as you are unlikely to have access to anything approaching the size of\n> the largest Git repository on the planet:\n> https://devblogs.microsoft.com/bharry/the-largest-git-repo-on-the-planet/\n> \n\nAh yes, I read about it a while back on Reddit. Having a huge monolithic \nrepo sounds backwards to me. Using submodules sounds like a better idea, \nbut who am I to judge. They probably have their reasons that I'm not \naware of.\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"379917","messageId":"nycvar.QRO.7.76.6.1908042048100.46@tvgsbejvaqbjf.bet","threadId":"51541","inReplyTo":"a143dcfd-8067-34f7-97f3-c4482dca5819@yadavpratyush.com","subject":"Re: [PATCH] git-gui: Perform rescan on window focus-in","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-08-04T18:56:03Z","receivedAt":"2019-08-04T19:01:31Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 3 Aug 2019, Pratyush Yadav wrote:\n\n> On 8/2/19 10:17 PM, Junio C Hamano wrote:\n> > Pratyush Yadav <me@yadavpratyush.com> writes:\n> >\n> > > All right, the patch in its current state can't fly. So what is\n> > > the correct way to do this? I see the following options:\n> > >\n> > > 1. Add this as an option that is disabled by default, but people\n> > > who don't mind it can enable it. This is the easiest to implement.\n> > > But I leave it to you and Junio (and anyone else who wants to\n> > > pitch in :)) to decide if it is a good idea.\n> >\n> > I think this is a good first step.  As I said already, I am not\n> > convinced that \"focus in\" is a good heuristics for triggering auto\n> > rescan, and I suspect that you or others may come up with and\n> > replace it with a better heuristic over time.  During that\n> > experiment, it would be better to allow interested others to opt\n> > into the feature to help, while not disturbing ordinary users who\n> > are OK with the current behaviour.\n> >\n>\n> All right. I'll look a bit more to see if I can figure something\n> better. I suggested using filesystem watches. I'll wait a bit to hear\n> from Johannes on what he thinks about it. He doesn't like the idea of\n> using Watchman apparently.\n\nIt's not that I don't like the idea of watchman. What I don't like is\nthe very limited number of scenarios where you can use watchman. It\nessentially boils down to... macOS.\n\nFrom what I can tell, watchman really works best on macOS, it might work\nwell on Linux, and it is in an \"eternal beta\" on Windows because none of\nthe active developers have access to (or need for support on) Windows.\n\nAlso, using a filesystem monitor is quite a lot of overkill here, you\nonly want to refresh the index smartly and pro-actively. Now you pile\nmore and more complexity on top of the original patch, for little\nobvious benefit (and for added difficulties setting this up, because you\ncan now no longer make this a default, as you have no idea whether\nwatchman is installed or not, whether it works as intended or not if it\nis installed, and whether users will start to hate you for forcing this\ndown their throats).\n\nI want you to take a step back, consider what problem you originally\ntried to solve, and then come up with the simplest solution you can\npossibly come up with. If your solution is not simple, reject it.\n\nI _could_ imagine, for example, that a focus-out event, followed by a\nconfigurable timeout (say, with the default being something like 10\nseconds, which seems to me like a sensible minimum number of seconds to\nchange anything commit-worthy), could be used by a focus-in event to\ntrigger a refresh (with a message in the status bar what is happening\nand why), and that that would strike a sensible balance between benefit\nand complexity.\n\nCiao,\nJohannes\n"},{"id":"379918","messageId":"nycvar.QRO.7.76.6.1908042058340.46@tvgsbejvaqbjf.bet","threadId":"51541","inReplyTo":"1489be05-ab18-a3e5-dd38-3d5729ebe67a@yadavpratyush.com","subject":"Re: [PATCH] git-gui: Perform rescan on window focus-in","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-08-04T19:10:13Z","receivedAt":"2019-08-04T19:10:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 4 Aug 2019, Pratyush Yadav wrote:\n\n> On 8/4/19 2:04 AM, Johannes Schindelin wrote:\n> >\n> > On Sat, 3 Aug 2019, Pratyush Yadav wrote:\n> >\n> > > On 8/2/19 6:09 PM, Johannes Schindelin wrote:\n> > > >\n> > > > On Fri, 2 Aug 2019, Pratyush Yadav wrote:\n> > > >\n> > > > > On 8/1/19 1:12 AM, Johannes Schindelin wrote:\n> > > > > >\n> > > > > > I would be _extremely_ cautious to base an argument on one\n> > > > > > particular setup, using on particular hardware with one\n> > > > > > particular OS and one particular repository.\n> > > > > >\n> > > > >\n> > > > > Agreed. That's why I asked for benchmarks from other people.\n> > > > > Unfortunately, no one replied.\n> > > >\n> > > > What stops _you_ from performing more tests yourself? There are\n> > > > tons of real-world repositories out there, we even talk about\n> > > > options for large repositories to test with in Git for Windows'\n> > > > Contributing Guidelines:\n> > > > https://github.com/git-for-windows/git/blob/master/CONTRIBUTING.md#performance-tests\n> > >\n> > > I thought the point was to not base all data off a single setup?\n> >\n> > You misunderstood what I was saying: a single setup is bad, and you\n> > can make it moderately better by testing _at least_ with a\n> > moderately-sized repository [*1*] in addition to git.git.\n> >\n> > So yes, it would still not be enough to test with, say, the git.git\n> > _and_ the Chromium repository _only_ on your setup, but if not even\n> > you can be bothered to test with more than one small repository, how\n> > can you possibly expect anybody else to add more testing?\n>\n> All right, I'll see what repos I can test.\n>\n> But my internet is pretty slow and unstable, so my clone of the\n> Chromium repo failed mid-way multiple times. I assume we need to test\n> on a large index, so is it all right if I use\n> t/perf/repos/many-files.sh to artificially generate a large repo?\n\nWhy do you ask me for permission to just try this? I feel very\nuncomfortable being put in such a position: I am not your manager or\ngate-keeper or anything.\n\n> > > [...]\n> > > > I wonder, however, whether you can think of a better method to\n> > > > figure out when to auto-refresh. Focus seems to be a low-hanging\n> > > > fruit, but as you noticed it is not very accurate. Maybe if you\n> > > > combine it with a timeout? Or maybe you can detect idle time in\n> > > > Tcl/Tk?\n> > >\n> > > Hm, I don't see a better alternative than file system watches.\n> > > Timeouts are a heuristic that can potentially be problematic.\n> >\n> > Let me stress the fact that I suggested a timeout _in addition_ to the\n> > focus event?\n>\n> Oh, my bad. I thought you suggested using timeouts exclusively.\n>\n> But I'm not sure I understand what you mean by \"using timeouts in addition to\n> the focus event\". My guess is that you mean we should activate a\n> refresh-on-focus-in only after git-gui has been out of focus for a certain\n> amount of time. Is my guess correct?\n\nI am _not_ telling you what strategy you should use. You really need to\ncome up with hypotheses about what tell-tales for committable outside\nchanges could be easy to detect. This is your patch, and your project.\n\nMy suggestion about a time-out was to think a bit further than just mere\nTk-provided events to detect whether the user might have changed\nanything outside of Git GUI that might make an automatic refresh\nconvenient for the user.\n\nI do _not_ want to engage in this project, it is not my pet peeve.\n\n> > Yes, using a timeout on its own is stupidly problematic. That's why I\n> > did not suggest that.\n> >\n> > > If you do a refresh too frequently, you hog up the user's resources\n> > > for little benefit.\n> >\n> > Indeed. You want to find a heuristic that catches most of the cases\n> > where files were changed, while at the same time not even _trying_ to\n> > refresh automatically when probably nothing changed.\n>\n> Like I said before, the best way of doing that that I can see is file system\n> watches.\n\nThat's not a heuristic.\n\nA file system monitor is doing a lot of work in this case, for dubitable\nbenefit.\n\nTake for example git.git: Let's say that I run a specific test. It\ncreates a directory under `t/`: the filesystem monitor triggers. It\ncreates a repository in that directory: the filesystem monitor triggers\n_multiple times_. The test then performs all kinds of I/O, eventually\nremoving the directory when all tests passed.\n\nNote that none of these filesystem changes correspond to anything that\nwould update _anything in Git GUI during a refresh.\n\nOf course, this is something I did not mention before because I took it\nfor granted that you would always try to weigh the benefits of your\napproach to the worst possible unintended consequences.\n\n> But maybe we can get reasonable performance with a combination of\n> timeouts and focus events.\n\nPlease note that I would not be surprised if this heuristic _also_\nresulted in a lot of bad, unintended consequences. That's for you to\nfind out.\n\n> > Footnote *1*: I don't expect you to test with the largest repositories,\n> > as you are unlikely to have access to anything approaching the size of\n> > the largest Git repository on the planet:\n> > https://devblogs.microsoft.com/bharry/the-largest-git-repo-on-the-planet/\n> >\n>\n> Ah yes, I read about it a while back on Reddit. Having a huge monolithic repo\n> sounds backwards to me. Using submodules sounds like a better idea, but who am\n> I to judge. They probably have their reasons that I'm not aware of.\n\nThis statement just sounds to me as if you never have used submodules in\nany serious way. My experience is that software developers who tried to\nuse submodules offer opinions that read very differently from that\nparagraph.\n\nStrong opinions usually do not survive contact with open-minded exposure\nto reality.\n\nCiao,\nJohannes\n"},{"id":"379920","messageId":"ea3d442d-2bda-8c2a-0f75-c9b6e32c6816@yadavpratyush.com","threadId":"51541","inReplyTo":"nycvar.QRO.7.76.6.1908042058340.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] git-gui: Perform rescan on window focus-in","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-08-04T20:17:21Z","receivedAt":"2019-08-04T20:17:35Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"Hi Johannes,\n\nI sense that I'm starting to test your patience. So first off, thanks \nfor the advice you've given so far. It has been really valuable. Feel \nfree to let me know if you don't want me to prod you further about this, \nand I'll stop :).\n\nAlso let me know if you don't want me to cc you in other git-gui patches \nI send.\n\n\nOn 8/5/19 12:40 AM, Johannes Schindelin wrote:\n> Hi,\n> \n> On Sun, 4 Aug 2019, Pratyush Yadav wrote:\n> \n>> On 8/4/19 2:04 AM, Johannes Schindelin wrote:\n>>>\n>>> On Sat, 3 Aug 2019, Pratyush Yadav wrote:\n>>>\n[...]\n>>>\n>>> You misunderstood what I was saying: a single setup is bad, and you\n>>> can make it moderately better by testing _at least_ with a\n>>> moderately-sized repository [*1*] in addition to git.git.\n>>>\n>>> So yes, it would still not be enough to test with, say, the git.git\n>>> _and_ the Chromium repository _only_ on your setup, but if not even\n>>> you can be bothered to test with more than one small repository, how\n>>> can you possibly expect anybody else to add more testing?\n>>\n>> All right, I'll see what repos I can test.\n>>\n>> But my internet is pretty slow and unstable, so my clone of the\n>> Chromium repo failed mid-way multiple times. I assume we need to test\n>> on a large index, so is it all right if I use\n>> t/perf/repos/many-files.sh to artificially generate a large repo?\n> \n> Why do you ask me for permission to just try this? I feel very\n> uncomfortable being put in such a position: I am not your manager or\n> gate-keeper or anything.\n\nI'm not a native speaker, so maybe I worded it incorrectly. What I meant \nby the question was not to ask permission, but to verify that this would \nbe a good test for checking performance with a large repo.\n\nSo I'll rephrase the question:\nWill using t/perf/repos/many-files.sh, instead of something like the \nChromium repo, be a good enough test of performance of a rescan?\n\n> \n>>>> [...]\n>>>>> I wonder, however, whether you can think of a better method to\n>>>>> figure out when to auto-refresh. Focus seems to be a low-hanging\n>>>>> fruit, but as you noticed it is not very accurate. Maybe if you\n>>>>> combine it with a timeout? Or maybe you can detect idle time in\n>>>>> Tcl/Tk?\n>>>>\n>>>> Hm, I don't see a better alternative than file system watches.\n>>>> Timeouts are a heuristic that can potentially be problematic.\n>>>\n>>> Let me stress the fact that I suggested a timeout _in addition_ to the\n>>> focus event?\n>>\n>> Oh, my bad. I thought you suggested using timeouts exclusively.\n>>\n>> But I'm not sure I understand what you mean by \"using timeouts in addition to\n>> the focus event\". My guess is that you mean we should activate a\n>> refresh-on-focus-in only after git-gui has been out of focus for a certain\n>> amount of time. Is my guess correct?\n> \n> I am _not_ telling you what strategy you should use. You really need to\n> come up with hypotheses about what tell-tales for committable outside\n> changes could be easy to detect. This is your patch, and your project.\n\nIDEs and some text editors like Atom use file system watches, and that's \nwhy I think they are a viable approach. But at the same time, these are \nalso known to be kind of heavy-weight and slow, and I like git-gui for \nbeing fast and lightweight, so that might not be a good idea.\n\n> \n> My suggestion about a time-out was to think a bit further than just mere\n> Tk-provided events to detect whether the user might have changed\n> anything outside of Git GUI that might make an automatic refresh\n> convenient for the user.\n> \n> I do _not_ want to engage in this project, it is not my pet peeve.\nUnderstandable. We are all volunteers after all. Thanks for your help so \nfar :)\n\n> \n>>> Yes, using a timeout on its own is stupidly problematic. That's why I\n>>> did not suggest that.\n>>>\n>>>> If you do a refresh too frequently, you hog up the user's resources\n>>>> for little benefit.\n>>>\n>>> Indeed. You want to find a heuristic that catches most of the cases\n>>> where files were changed, while at the same time not even _trying_ to\n>>> refresh automatically when probably nothing changed.\n>>\n>> Like I said before, the best way of doing that that I can see is file system\n>> watches.\n> \n> That's not a heuristic.\n> \n> A file system monitor is doing a lot of work in this case, for dubitable\n> benefit.\n> \n> Take for example git.git: Let's say that I run a specific test. It\n> creates a directory under `t/`: the filesystem monitor triggers. It\n> creates a repository in that directory: the filesystem monitor triggers\n> _multiple times_. The test then performs all kinds of I/O, eventually\n> removing the directory when all tests passed.\n\nThe monitor triggers multiple times, but we don't have to do a rescan \nevery time. The callback for the trigger would just set a boolean that \nwill decide whether a rescan has to be done the next time it comes back \ninto focus.\n\n> \n> Note that none of these filesystem changes correspond to anything that\n> would update _anything in Git GUI during a refresh.\n> \n> Of course, this is something I did not mention before because I took it\n> for granted that you would always try to weigh the benefits of your\n> approach to the worst possible unintended consequences.\n\nYes, the specific use case you mentioned would trigger an unnecessary \nrefresh, but I don't think many people do these kinds of things very often.\n\nBuilding the tree will generate a lot of IO, but we will only watch \nfiles tracked by git, so all that IO would not trigger a refresh at all.\n\nI'm not saying file system watches are the best alternative here, I'm \njust saying it is a pretty good alternative, and the scenario you \ndescribe doesn't happen often enough to bother most people. One problem \nwith them is, of course, added dependencies. And it's a big problem. I'd \nlove to have a tcl-only alternative, but there is none at the moment.\n\nI'll experiment with some file system watches, and see how they fare.\n\n> \n>> But maybe we can get reasonable performance with a combination of\n>> timeouts and focus events.\n> \n> Please note that I would not be surprised if this heuristic _also_\n> resulted in a lot of bad, unintended consequences. That's for you to\n> find out.\n\nYes, of course. I was just thinking out loud.\n\n> \n>>> Footnote *1*: I don't expect you to test with the largest repositories,\n>>> as you are unlikely to have access to anything approaching the size of\n>>> the largest Git repository on the planet:\n>>> https://devblogs.microsoft.com/bharry/the-largest-git-repo-on-the-planet/\n>>>\n>>\n>> Ah yes, I read about it a while back on Reddit. Having a huge monolithic repo\n>> sounds backwards to me. Using submodules sounds like a better idea, but who am\n>> I to judge. They probably have their reasons that I'm not aware of.\n> \n> This statement just sounds to me as if you never have used submodules in\n> any serious way. My experience is that software developers who tried to\n> use submodules offer opinions that read very differently from that\n> paragraph.\n\nMy last workplace had Linux, U-Boot, ARM Trusted Firmware, and a handful \nmore projects under one repo with submodules. I don't remember having \nany problems, and hence why I thought it was a pretty decent idea.\n\n> \n> Strong opinions usually do not survive contact with open-minded exposure\n> to reality.\n\n\"but who am I to judge. They probably have their reasons that I'm not \naware of.\"\n\nI just put my first thoughts out about this, admitting I probably don't \nhave the full picture. I see that as being open-minded.\n\n-- \nRegards,\nPratyush Yadav\n"}]}