Allow toggling patch checkbox by clicking on lines

[simonwiles]

This PR changes the line-select UI in patch mode to allow toggling lines by clicking anywhere on the line. It doesn't address all the desired improvements described in #1421 (and so won't close it), but I've been using this modification for a while and it does make things somewhat easier.

If it passes an initial review I can update the CHANGELOG.md and bump the version number.

[wmertens]

Would it be possible as a quick win to add a button for toggling all? Just an idea, not worth blocking this over.

[simonwiles]

Would it be possible as a quick win to add a button for toggling all? Just an idea, not worth blocking this over.

Yeah, I like the idea of an "invert selection" button, or something like that. I'm not going to be able to do it right now, but I'll make a note. I might look at addressing actual drag selecting if I can find time too, but I'd rather see this closed off first, because with the best will in the world I don't know if/when I'll have time!

Knockout novice but LGTM

Likewise, but tbh this is more about getting the regex find/replace right :)

Is the advice at https://github.com/FredrikNoren/ungit/blob/master/CONTRIBUTING.md#pull-requests still valid? I notice the release process seems to have changed a bit since this was written. I've updated the changelog under "unreleased" and not touched the version number in package.json -- hope that's okay.

[simonwiles]

The numberOfSelectedPatchLines variable was only used to reset the edit state when it evaluated to zero, so I dropped it and replace the check with a test against this.patchLineList().filter(Boolean).length (see 825e120).

[simonwiles]

Important:

When investigating this, I noticed a bug in the way that ungit operates in the present release version (1.5.11). See below for a quick demonstration:

The reason this occurs is that after all the checkboxes are cleared, the edit state is set to none, and they are hidden; when the patch button is clicked again, the array returned by patchLineList() is reinitialized to all true, but the UI is not re-rendered, so the boxes all appear empty. Clicking one inverts its state, and so what gets committed as the patch is the inverse of the UI state. Note too that not moving the mouse for 3 seconds and then moving it again causes the UI to re-render (because of https://github.com/FredrikNoren/ungit/blob/9374f6c0f6dafc6dd8f9240eb506a087e1fa39ce/public/source/main.js#L115-L125):

This became much more obvious with the earlier commits in this PR, because the diff (and checkboxes) are re-rendered on click:

My choice to address this was to make the diff re-render on each render (i.e. not checking for a difference in the html string, as this check is inadequate to determine if the state of the checkboxes has changed anyway). This doesn't generate more re-renders or cause any thrashing, and fixes the issue.

[simonwiles]

Okay, this last commit addresses the bug in production, I think (although it's really a workaround rather than a proper fix).

[wmertens]

I wish that you could click and drag (or tap + hold and drag) to select/unselect lines, and that there was a toggle select button like the toggle files button, but this PR is already a great improvement and I'd rather not wait for those improvements.

@campersau I tried it out, it merged cleanly and LGTM, would you consider merging?

[simonwiles]

I'm just getting around to rebasing all my local changes on top of the most recent release, and I realized this PR is still open. Might there be any interest in revisiting this?