Re: [PATCH] git-gui: accommodate for intent-to-add files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Pratyush,

On Wed, 26 Aug 2020, Pratyush Yadav wrote:

> On 26/08/20 09:36AM, Johannes Schindelin wrote:
>
> > On Wed, 26 Aug 2020, Pratyush Yadav wrote:
> >
> > > On 12/08/20 03:06PM, Johannes Schindelin via GitGitGadget wrote:
> > > > From: Johannes Schindelin <johannes.schindelin@xxxxxx>
> > > >
> > > > As of Git v2.28.0, the diff for files staged via `git add -N` marks them
> > > > as new files. Git GUI was ill-prepared for that, and this patch teaches
> > > > Git GUI about them.
> > > >
> > > > Please note that this will not even fix things with v2.28.0, as the
> > > > `rp/apply-cached-with-i-t-a` patches are required on Git's side, too.
> > > >
> > > > This fixes https://github.com/git-for-windows/git/issues/2779
> > > >
> > > > Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> > > > ---
> > > >     git-gui: accommodate for intent-to-add files
> > > >
> > > >     This fixes the intent-to-add bug reported in
> > > >     https://github.com/git-for-windows/git/issues/2779: after a file was
> > > >     staged with git add -N, staging hunks/lines would fail silently.
> > > >
> > > >     On its own, this patch is not enough, as it requires the patches
> > > >     provided in rp/apply-cached-with-i-t-a to be applied on Git's side.
> > > >
> > > >     Please note that this patch might need a bit more help, as I do not
> > > >     really know whether showing "new file mode 100644" in the diff view is
> > > >     desirable, or whether we should somehow try to retain the
> > > >     "intent-to-add" state so that unstaging all hunks would return the file
> > > >     to "intent-to-add" state.
> > >
> > > I built latest Git master (e9b77c84a0) which has
> > > `rp/apply-cached-with-i-t-a` and tested this patch. It works... for the
> > > most part.
> > >
> > > I can select a line set of lines and they get staged/unstaged, which is
> > > good. The part that is not good though is that a lot of common
> > > operations still don't work as they should:
> > >
> > > - I can't click on the icon in the "Unstaged Changes" pane to stage the
> > >   whole file. Nothing happens when I do that.
> > >
> > > - The file that is marked as intent-to-add shows up in both the "Staged
> > >   Changes" and "Unstaged Changes" panes, with the "Staged Changes" part
> > >   being empty. Ideally it should only show up in the "Unstaged Changes"
> > >   pane.
> > >
> > > - Selecting the whole file and choosing "Stage Lines for Commit" works
> > >   well. But choosing "Stage Hunk for Commit" does not. While the changes
> > >   do get staged, the UI is not properly updated and the file is still
> > >   listed in the "Unstaged Changes" pane.
> > >
> > >   I think the difference here is because for
> > >   `apply_or_revert_range_or_line`, we call `do_rescan` after it to
> > >   update the UI, but for `apply_or_revert_hunk` we update the UI
> > >   "manually" in the function after we are done applying or reverting the
> > >   changes. So the logic to update the UI needs to be updated to account
> > >   for this change. Or we can get rid of all that logic and just run a
> > >   rescan.
> > >
> > > And also, like you mentioned, we don't retain the i-t-a state when
> > > unstaging. But with some quick testing, I see that Git command line
> > > doesn't either (I tried a plain `git restore --staged`). So IMO we
> > > should mimic what the command line UI does and not retain the i-t-a
> > > state when unstaging.
> >
> > To be quite honest, I had hoped that this might be a good patch to start
> > from... for somebody else (you?)
>
> I'll take a stab at this during the weekend :-)

Just a gentle ping: did you get a chance to get this patch into a better
shape?

Thanks,
Dscho




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux