git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Pratyush Yadav <me@yadavpratyush.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	Mark Levedahl <mlevedahl@gmail.com>,
	"brian m. carlson" <sandals@crustytoothpaste.net>,
	git <git@vger.kernel.org>,
	Christian Couder <christian.couder@gmail.com>
Subject: Re: [PATCH] git-gui: Perform rescan on window focus-in
Date: Sun, 4 Aug 2019 20:56:03 +0200 (CEST)	[thread overview]
Message-ID: <nycvar.QRO.7.76.6.1908042048100.46@tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <a143dcfd-8067-34f7-97f3-c4482dca5819@yadavpratyush.com>

Hi,

On Sat, 3 Aug 2019, Pratyush Yadav wrote:

> On 8/2/19 10:17 PM, Junio C Hamano wrote:
> > Pratyush Yadav <me@yadavpratyush.com> writes:
> >
> > > All right, the patch in its current state can't fly. So what is
> > > the correct way to do this? I see the following options:
> > >
> > > 1. Add this as an option that is disabled by default, but people
> > > who don't mind it can enable it. This is the easiest to implement.
> > > But I leave it to you and Junio (and anyone else who wants to
> > > pitch in :)) to decide if it is a good idea.
> >
> > I think this is a good first step.  As I said already, I am not
> > convinced that "focus in" is a good heuristics for triggering auto
> > rescan, and I suspect that you or others may come up with and
> > replace it with a better heuristic over time.  During that
> > experiment, it would be better to allow interested others to opt
> > into the feature to help, while not disturbing ordinary users who
> > are OK with the current behaviour.
> >
>
> All right. I'll look a bit more to see if I can figure something
> better. I suggested using filesystem watches. I'll wait a bit to hear
> from Johannes on what he thinks about it. He doesn't like the idea of
> using Watchman apparently.

It's not that I don't like the idea of watchman. What I don't like is
the very limited number of scenarios where you can use watchman. It
essentially boils down to... macOS.

From what I can tell, watchman really works best on macOS, it might work
well on Linux, and it is in an "eternal beta" on Windows because none of
the active developers have access to (or need for support on) Windows.

Also, using a filesystem monitor is quite a lot of overkill here, you
only want to refresh the index smartly and pro-actively. Now you pile
more and more complexity on top of the original patch, for little
obvious benefit (and for added difficulties setting this up, because you
can now no longer make this a default, as you have no idea whether
watchman is installed or not, whether it works as intended or not if it
is installed, and whether users will start to hate you for forcing this
down their throats).

I want you to take a step back, consider what problem you originally
tried to solve, and then come up with the simplest solution you can
possibly come up with. If your solution is not simple, reject it.

I _could_ imagine, for example, that a focus-out event, followed by a
configurable timeout (say, with the default being something like 10
seconds, which seems to me like a sensible minimum number of seconds to
change anything commit-worthy), could be used by a focus-in event to
trigger a refresh (with a message in the status bar what is happening
and why), and that that would strike a sensible balance between benefit
and complexity.

Ciao,
Johannes

  reply	other threads:[~2019-08-04 19:01 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-28 15:17 Pratyush Yadav
2019-07-28 21:36 ` brian m. carlson
2019-07-28 22:10   ` Pratyush Yadav
2019-07-28 22:32     ` Pratyush Yadav
2019-07-28 22:49     ` brian m. carlson
2019-07-29  2:24       ` Mark Levedahl
2019-07-29  2:26       ` Mark Levedahl
2019-07-29  2:28       ` Mark Levedahl
2019-07-29  8:15         ` Pratyush Yadav
2019-07-31 19:42           ` Johannes Schindelin
2019-08-01 21:52             ` Pratyush Yadav
2019-08-02 12:39               ` Johannes Schindelin
2019-08-02 20:00                 ` Pratyush Yadav
2019-08-03 20:34                   ` Johannes Schindelin
2019-08-04 12:53                     ` Pratyush Yadav
2019-08-04 19:10                       ` Johannes Schindelin
2019-08-04 20:17                         ` Pratyush Yadav
2019-08-02 16:47               ` Junio C Hamano
2019-08-02 20:13                 ` Pratyush Yadav
2019-08-04 18:56                   ` Johannes Schindelin [this message]
2019-07-29  5:09       ` Junio C Hamano
2019-07-29  8:44         ` Pratyush Yadav
2019-07-28 21:44 ` Pratyush Yadav

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=nycvar.QRO.7.76.6.1908042048100.46@tvgsbejvaqbjf.bet \
    --to=johannes.schindelin@gmx.de \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=me@yadavpratyush.com \
    --cc=mlevedahl@gmail.com \
    --cc=sandals@crustytoothpaste.net \
    --subject='Re: [PATCH] git-gui: Perform rescan on window focus-in' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).