git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
From: Jonathan Gilbert <logic@deltaq.org>
To: "Pratyush Yadav me-at-yadavpratyush.com |GitHub Public/Example
	Allow|"  <172q77k4bxwj0zt@sneakemail.com>
Cc: Jonathan Gilbert via GitGitGadget <gitgitgadget@gmail.com>,
	Git Mailing List <git@vger.kernel.org>,
	Jonathan Gilbert <rcq8n2xf3v@liamekaens.com>,
	Jonathan Gilbert <JonathanG@iqmetrix.com>
Subject: Re: [PATCH v6 2/3] git-gui: update status bar to track operations
Date: Sat, 30 Nov 2019 20:12:55 -0600
Message-ID: <CAPSOpYs7GeKhcWLHtmwuXQWimp-Bgk8F5WYoO+XQM8C8dDj7pA@mail.gmail.com> (raw)
In-Reply-To: <20191130230543.p5xtapnx5a56arng@yadavpratyush.com>

On Sat, Nov 30, 2019 at 5:05 PM Pratyush Yadav me-at-yadavpratyush.com
|GitHub Public/Example Allow| <172q77k4bxwj0zt@sneakemail.com> wrote:
> Hi Jonathan,
>
> Thanks for the re-roll.

You are most welcome :-)

> On 28/11/19 08:30AM, Jonathan Gilbert via GitGitGadget wrote:
> > +# Operation displayed by status mega-widget during _do_clone_checkout =>
> > +# _readtree_wait => _postcheckout_wait => _do_clone_submodules =>
> > +# _do_validate_submodule_cloning. The status mega-widget is a difference
> > +# instance than that stored in $o_status in earlier operations.
>
> The last sentence doesn't make a lot of sense to me. What is "earlier
> operations"? If this refers to previous versions of this file, then I
> don't think such a comment belongs here. It should be in the commit
> message instead.

A clone starts out by calling `_do_clone2`, which, for `$clone_type`
of `hardlink`, creates a status "mega-widget" and uses it to track
linking and/or copying the underlying files. Then, this part of the UI
is destroyed. Later, the code calls into _do_clone_checkout, which
sets up its own, different view. This view _also_ uses a status
"mega-widget", but it's not the same one as before. This wasn't
obvious to me in my first read-through, and I erroneously wrote code
that assumed the widget objects would carry forward. As such, I felt
it might be useful to other readers to have this detail called out
up-front. In the context of `_do_clone_checkout`, the "earlier
operations" is what happens in `_do_clone2`.

> >               destroy $w_body
> > +
> > +             set o_status {}
>
> Should we be calling a destructor for this here? There is the '_delete'
> method in status_bar.tcl, but I don't see any usages of it so I'm not
> sure what exactly it is supposed to do.
>
> That said, the previous version of this file doesn't call any sort of
> destructor either, so maybe we should just leave it like it is for now.
> I dunno.

As far as I can tell, `destroy $w_body` automatically deletes the
entire subtree of UI components. I mentioned that I had written broken
code at first because I didn't realize the status widget got replaced
between `_do_clone2` and `_do_clone_checkout` -- that code encountered
an error that indicated that the status widget object no longer
existed at all. Thus, I have proceeded on the assumption that `destroy
$w_body` handles that particular detail, and all that's left is to
clear `o_status` of its dangling reference to the object that no
longer exists.

> > -method _do_validate_submodule_cloning {ok} {
> > [..]
> > -method _do_clone_submodules {} {
>
> Is there a reason for moving these two methods around? Not that its a
> bad thing, I'm just curious.

I touched on this in the cover letter. I'll just copy/paste that text
since it says it just as well as I could re-synthesize here :-)

* In `choose_repository.tcl`, there is a sequence of functions
involved performing the checkout on the clone: `_do_clone_checkout` =>
`_readtree_wait` => `_postcheckout_wait` => `_do_clone_submodules` =>
`_do_validate_submodule_cloning`. The functions have been re-ordered
in the source code to match the sequence in which they execute to
improve clarity.

Re-roll (final?) incoming.

Thanks,

Jonathan Gilbert

  reply index

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-30  6:48 [PATCH 0/2] git-gui: revert untracked files by deleting them Jonathan Gilbert via GitGitGadget
2019-10-30  6:48 ` [PATCH 1/2] git-gui: consolidate naming conventions Jonathan Gilbert via GitGitGadget
2019-11-03  0:27   ` Pratyush Yadav
2019-10-30  6:48 ` [PATCH 2/2] git-gui: revert untracked files by deleting them Jonathan Gilbert via GitGitGadget
2019-11-03  7:44   ` Pratyush Yadav
2019-11-04 16:04     ` Jonathan Gilbert
2019-11-04 17:36     ` Jonathan Gilbert
2019-10-30  9:06 ` [PATCH 0/2] " Bert Wesarg
2019-10-30 17:16   ` Jonathan Gilbert
2019-11-03  1:12     ` Pratyush Yadav
2019-11-03  4:41       ` Jonathan Gilbert
2019-11-03  7:54         ` Pratyush Yadav
2019-11-07  7:05 ` [PATCH v2 " Jonathan Gilbert via GitGitGadget
2019-11-07  7:05   ` [PATCH v2 1/2] git-gui: consolidate naming conventions Jonathan Gilbert via GitGitGadget
2019-11-07  7:05   ` [PATCH v2 2/2] git-gui: revert untracked files by deleting them Jonathan Gilbert via GitGitGadget
2019-11-11 19:25     ` Pratyush Yadav
2019-11-11 21:55       ` Jonathan Gilbert
2019-11-11 22:59         ` Philip Oakley
2019-11-12  4:49           ` Jonathan Gilbert
2019-11-12 10:45             ` Philip Oakley
2019-11-12 16:29               ` Jonathan Gilbert
2019-11-26 11:22                 ` Philip Oakley
2019-11-12 19:35         ` Pratyush Yadav
2019-11-11 19:35   ` [PATCH v2 0/2] " Pratyush Yadav
2019-11-13  9:56   ` [PATCH v3 " Jonathan Gilbert via GitGitGadget
2019-11-13  9:56     ` [PATCH v3 1/2] git-gui: consolidate naming conventions Jonathan Gilbert via GitGitGadget
2019-11-13  9:56     ` [PATCH v3 2/2] git-gui: revert untracked files by deleting them Jonathan Gilbert via GitGitGadget
2019-11-16 15:11       ` Pratyush Yadav
2019-11-16 21:42         ` Jonathan Gilbert
2019-11-17  6:56     ` [PATCH v4 0/2] " Jonathan Gilbert via GitGitGadget
2019-11-17  6:56       ` [PATCH v4 1/2] git-gui: consolidate naming conventions Jonathan Gilbert via GitGitGadget
2019-11-17  6:56       ` [PATCH v4 2/2] git-gui: revert untracked files by deleting them Jonathan Gilbert via GitGitGadget
2019-11-24 13:09         ` Pratyush Yadav
2019-11-19 15:21       ` [PATCH v4 0/2] " Pratyush Yadav
2019-11-19 16:56         ` Jonathan Gilbert
2019-11-24 20:37       ` [PATCH v5 0/3] " Jonathan Gilbert via GitGitGadget
2019-11-24 20:37         ` [PATCH v5 1/3] git-gui: consolidate naming conventions Jonathan Gilbert via GitGitGadget
2019-11-24 20:37         ` [PATCH v5 2/3] git-gui: update status bar to track operations Jonathan Gilbert via GitGitGadget
2019-11-27 21:55           ` Pratyush Yadav
2019-11-28  7:34             ` Jonathan Gilbert
2019-11-24 20:37         ` [PATCH v5 3/3] git-gui: revert untracked files by deleting them Jonathan Gilbert via GitGitGadget
2019-11-27 22:03           ` Pratyush Yadav
2019-11-28  8:30         ` [PATCH v6 0/3] " Jonathan Gilbert via GitGitGadget
2019-11-28  8:30           ` [PATCH v6 1/3] git-gui: consolidate naming conventions Jonathan Gilbert via GitGitGadget
2019-11-28  8:30           ` [PATCH v6 2/3] git-gui: update status bar to track operations Jonathan Gilbert via GitGitGadget
2019-11-30 23:05             ` Pratyush Yadav
2019-12-01  2:12               ` Jonathan Gilbert [this message]
2019-12-01 11:43               ` Philip Oakley
2019-12-01 20:09                 ` Jonathan Gilbert
2019-11-28  8:30           ` [PATCH v6 3/3] git-gui: revert untracked files by deleting them Jonathan Gilbert via GitGitGadget
2019-12-01  2:28           ` [PATCH v7 0/3] " Jonathan Gilbert via GitGitGadget
2019-12-01  2:28             ` [PATCH v7 1/3] git-gui: consolidate naming conventions Jonathan Gilbert via GitGitGadget
2019-12-01  2:28             ` [PATCH v7 2/3] git-gui: update status bar to track operations Jonathan Gilbert via GitGitGadget
2020-02-26  8:24               ` Benjamin Poirier
2020-03-02 18:14                 ` Pratyush Yadav
2019-12-01  2:28             ` [PATCH v7 3/3] git-gui: revert untracked files by deleting them Jonathan Gilbert via GitGitGadget
2019-12-05 18:54             ` [PATCH v7 0/3] " 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=CAPSOpYs7GeKhcWLHtmwuXQWimp-Bgk8F5WYoO+XQM8C8dDj7pA@mail.gmail.com \
    --to=logic@deltaq.org \
    --cc=172q77k4bxwj0zt@sneakemail.com \
    --cc=JonathanG@iqmetrix.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=rcq8n2xf3v@liamekaens.com \
    /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

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git