On Mon, 16 Dec 2019 at 20:39, Junio C Hamano <gitster@xxxxxxxxx> wrote: > > "Ben Keene via GitGitGadget" <gitgitgadget@xxxxxxxxx> 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.
