Re: [PATCH v5 0/4] git-p4: Usability enhancements

From: Luke Diamand <luke@diamand.org>
Date: Dec 21, 2019, 10:19 UTC
Message-ID: <CAE5ih7-ptmmb2UurBw+k+2ZjZQuOkLJ3c-eBoOXKrPX0CJeErA@mail.gmail.com>
In-Reply-To: <xmqq8sncj991.fsf@gitster-ct.c.googlers.com>

On Mon, 16 Dec 2019 at 20:39, Junio C Hamano <gitster@pobox.com> wrote:

Show 16 quoted lines

>
> "Ben Keene via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > Some user interaction with git-p4 is not as user-friendly as the rest of the
> > ...
> >
> > Ben Keene (4):
> >   git-p4: yes/no prompts should sanitize user text
> >   git-p4: show detailed help when parsing options fail
> >   git-p4: wrap patchRCSKeywords test to revert changes on failure
> >   git-p4: failure because of RCS keywords should show help
>
> The reviews on the list seem to be in favor of these and I didn't
> see much wrong (I think I fixed up an indented empty line) in the
> series.  I'd appreciate a blessing from a git-p4 expert, though.
>

$ git log --reverse --oneline --abbrev-commit
origin/maint..origin/bk/p4-misc-usability
e2aed5fd5b git-p4: yes/no prompts should sanitize user text
   - looks good to me

608e380502 git-p4: show detailed help when parsing options fail
   - also looks good to me

c4dc935311 git-p4: wrap patchRCSKeywords test to revert changes on failure
   - why not just catch the exception, and then drop out of the "if-"
condition and fall into the cleanup section at the bottom of that
function (line 1976)? As it stands, this is duplicating the cleanup
code now.

89c88c0ecf (origin/bk/p4-misc-usability) git-p4: failure because of
RCS keywords should show help
  - strictly speaking, the code does not actually check if there *are*
any RCS keywords, it just checks if the filetype means that RCS kws
*would* be expanded *if* they were present. The conflict might be just
because....there's a conflict. As it stands this will be giving
misleading advice. I would get it to check to see if there really are
any RCS keywords in the file.

> Thanks.