Re: [PATCH v2 2/2] git-gui: revert untracked files by deleting them

On Mon, Nov 11, 2019 at 1:25 PM Pratyush Yadav me-at-yadavpratyush.com
|GitHub Public/Example Allow| <172q77k4bxwj0zt@sneakemail.com> wrote:

I'm not super familiar with it. I just checked what Tcl version I was myself running, since it's only there because of the Git Gui installation bundled with Git for Windows, and it was 8.6, so I assumed it was fair game to use. It didn't occur to me that you could already have an older version of Tcl installed and have Git Gui use it. :-) So, if I'm understanding correctly, `TclOO` as a package could potentially be used to allow TclOO to be used with 8.4, the minimum supported version you mention below, and it just happened to work for me in my testing without that because I have 8.6 installed but that's technically newer than the supported baseline?

Hmm, what do you think of both? I was thinking of the documentation as a single self-contained block that someone could read to put together an understanding of how the chord system fits together, and split out, it wouldn't have that readability. What about a more abstract description in a block at the top, and then more technically-detailed & specific descriptions attached to each method?

Having limited experience with Tcl, I did a Google search for "tcl object oriented" and ended up writing code using TclOO because that's what came up. Do you think it makes sense to rework this to use `class.tcl`, or perhaps instead the opposite: have a policy of using the standard TclOO going forward, and let the rest of Git Gui organically upgrade itself to some hypothetical point in the future where class.tcl is no longer used by anything?

If I understand correctly, you mentioned that TclOO was intrinsically available to me because I was using Tcl 8.6, and that the manual recommends `package require TclOO` -- does that package dependency permit the use of TclOO on 8.4? If so, could that be a way to avoid bumping the minimum version required? Simply in the interest of keeping the scope of the change limited. If not, then bumping the minimum required version to 8.6 from 2012 doesn't seem entirely unreasonable either. :-)

Okay, yep -- I had copied the convention that I saw in TclOO examples, conscious of the fact that there might be a standard specific to object-oriented Tcl.

I don't think so, because it's called from outside the class. What we'd be looking for is something like C++'s "friend" syntax. Tcl doesn't seem to have this. Though, I just did some further Googling, and saw a hint that it might be possible to bypass member security on a case-by-case basis, so that the method is private but `ChordNote` is able to call it anyway. I'll see if I can't figure this out. :-)

I think it's the fact that it is named `unknown` that makes it the "default" method. I think this just needs documentary comments next to it. The TclOO documentation says:

It was based on that last sentence that I interpreted `unknown` as, "This is a mechanism for making an object that can be called like a method."

I learned about the concept of chords and notes from an experimental language that Microsoft created many years back called "Polyphonic C#" (which in turn got rolled into "Cw" (C-omega)), and in that abstraction, the idea was that, well, as a baseline, for starters, we have methods and each one, conceptually, has an entrypoint with a certain set of parameters, and when you call that entrypoint, the parameters are all set and the body runs. With a "chord", you have more than one entrypoint attached to the same body -- the entrypoints themselves don't have any logic associated with them individually. Each note has its own parameter list, and when all the notes have been called, the body is run with _all_ of those parameters.

I drew some ASCII art, don't know if it'll translate in the message, but here goes :-)

Basic method (or, if you will, a "chord" with only one "note"):

           (caller)
              |
    void Add(int X, int Y)
              |
      { output(X + Y) }

A "chord" with two "notes":

        (caller)                (caller)
            |                       |
    void AddX(int X)         void AddY(int Y)
            |                       |
            `-----------.-----------'
                        |
                { output(X + Y) }

The specific details differ from what I've written here. In Polyphonic C#, you don't have to instantiate a chord, you simply start calling methods, and the runtime matches up complete sets dynamically. (Just thinking through the implications of this, if the notes aren't all called at exactly the same rate this obviously leads very easily to bugs that chew up all memory on incomplete chords. :-P) Also, Microsoft's language has parameters to each of the notes that are _all_ passed to the body at once. My implementation here is a "simple" chord, I didn't bother with arguments, as they aren't needed in this usage :-) I also found it much simpler to think of implementing the chord with the activations being explicit instead of implicit. So instead of saying up front, "Here is my method body and here are its 3 entrypoints", with this implementation the chord is a dynamic object, you say "Here is my method body" and get back a thing that you can start tacking entrypoints onto.

But, a "note" in a SimpleChord isn't a counter, it's a latch. The chord itself is acting sort of like a counter, in that all the notes need to be activated, but because the notes are latches, activating a note repeatedly has the same effect as activating it once. There's no way for one note to interfere with other notes, which wouldn't be the case if it literally were just a counter.

It seems to me that a chord where each note has a script of its own is actually basically just a class with methods, I guess with a common joined epilogue?:

        (caller)                (caller)
            |                       |
    void AddX(int X)         void AddY(int Y)
            |                       |
   { script for AddX }      {script for AddY }
            |                       |
            `-----------.-----------'
                        |
                { common tail?? }

The whole point is that the notes are conceptually different "headers" into _the same_ body. When you call a note of a chord, it is because you want the _chord_'s script to run, and the chord is acting as a construct that says "okay, yes, I'll satisfy your request that I execute, but you'll have to wait, because I'm going to satisfy _all_ your requests in one go".

Hmm, so, yeah, the entire if statement only occurs if it can't close the file descriptor. Is that something that actually happens? If so, then it should perhaps be throwing an exception, because having started a rescan is probably more than the caller bargained for. That would prevent the callers from unlocking the index out from under the rescan, and also cancel any other processing they might be doing that is probably making bad assumptions with a rescan running.

It is indeed, oops!

Intentional, yes. I'll see if there's a concise way to document this.

Okeydoke.

Good catch.

Will do.

Ah, yes, I did that with most messages, this was an oversight.

A variable is by definition not a constant. This is the pattern that came up when I did a search for how one makes a constant in Tcl. ""\_( ``_/ )_/""

Making it a procedure means that if someone wants to put actual logic behind it in the future, it's already being called as a proc.

Yeah, I did run things as I was changing them to verify, and felt like I covered everything, I'm surprised I didn't bump into that, obviously I didn't cover everything after all. Perfect demonstration of why developers should never be exclusively responsible for testing their own code :-D

Let me know w.r.t. which OO framework to employ and what that means for minimum required versions and/or package references.

Thanks very much,

Jonathan Gilbert