Subject: [PATCH v2 0/4] git-p4: Usability enhancements

From: "Ben Keene via GitGitGadget" <[email-masked]>
Date: Tue, 10 Dec 2019 15:22:50 +0000

Some user interaction with git-p4 is not as user-friendly as the rest of the
Git ecosystem. Here are three areas that can be improved on:

1) When a patch fails and the user is prompted, there is no sanitization of
the user input so for a "yes/no" question, if the user enters "YES" instead
of a lowercase "y", they will be re-prompted to enter their answer. 

Commit 1 addresses this by sanitizing the user text by trimming and
lowercasing their input before testing. Now "YES" will succeed!

2) Git can handle scraping the RCS Keyword expansions out of source files
when it is preparing to submit them to P4. However, if the config value
"git-p4.attemptRCSCleanup" isn't set, it will just report that it fails.

Commit 2 adds a helpful suggestion, that the user might want to set
git-p4.attemptRCSCleanup.

3) If the command line arguments are incorrect for git-p4, the program
reports that there was a syntax error, but doesn't show what the correct
syntax is.

Commit 3 displays the context help for the failed command.

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

 git-p4.py | 43 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 37 insertions(+), 6 deletions(-)

base-commit: 083378cc35c4dbcc607e4cdd24a5fca440163d17
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-675%2Fseraphire%2Fseraphire%2Fp4-usability-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-675/seraphire/seraphire/p4-usability-v2
Pull-Request: https://github.com/git/git/pull/675

Range-diff vs v1:

 1:  e721cdaa00 ! 1:  527b7b8f8a git-p4: [usability] yes/no prompts should sanitize user text
     @@ -1,6 +1,6 @@
      Author: Ben Keene <[email-masked]>
      
     -    git-p4: [usability] yes/no prompts should sanitize user text
     +    git-p4: yes/no prompts should sanitize user text
      
          When prompting the user interactively for direction, the tests are
          not forgiving of user input format.
 3:  2a10890ef7 ! 2:  1d4f4e210b git-p4: [usability] Show detailed help when parsing options fail
     @@ -1,6 +1,6 @@
      Author: Ben Keene <[email-masked]>
      
     -    git-p4: [usability] Show detailed help when parsing options fail
     +    git-p4: show detailed help when parsing options fail
      
          When a user provides invalid parameters to git-p4, the program
          reports the failure but does not provide the correct command syntax.
 2:  d608f529a0 ! 3:  20aa557193 git-p4: [usability] RCS Keyword failure should suggest help
     @@ -1,14 +1,13 @@
      Author: Ben Keene <[email-masked]>
      
     -    git-p4: [usability] RCS Keyword failure should suggest help
     +    git-p4: wrap patchRCSKeywords test to revert changes on failure
      
     -    When applying a commit fails because of RCS keywords, Git
     -    will fail the P4 submit. It would help the user if Git suggested that
     -    the user set git-p4.attemptRCSCleanup to true.
     +    The patchRCSKeywords function has the potentional of throwing
     +    an exception and this would leave files checked out in P4 and partially
     +    modified.
      
     -    Change the applyCommit() method that when applying a commit fails
     -    becasue of the P4 RCS Keywords, the user should consider setting
     -    git-p4.attemptRCSCleanup to true.
     +    Add a try-catch block around the patchRCSKeywords call and revert
     +    the edited files in P4 before leaving the method.
      
          Signed-off-by: Ben Keene <[email-masked]>
      
     @@ -30,14 +29,6 @@
      +                        for f in editedFiles:
      +                            p4_revert(f)
      +                        raise
     -+            else:
     -+                # They do not have attemptRCSCleanup set, this might be the fail point
     -+                # Check to see if the file has RCS keywords and suggest setting the property.
     -+                for file in editedFiles | filesToDelete:
     -+                    if p4_keywords_regexp_for_file(file) != None:
     -+                        print("At least one file in this commit has RCS Keywords that may be causing problems. ")
     -+                        print("Consider:\ngit config git-p4.attemptRCSCleanup true")
     -+                        break
       
                   if fixed_rcs_keywords:
                       print("Retrying the patch with RCS keywords cleaned up")
 -:  ---------- > 4:  50e9a175c3 git-p4: failure because of RCS keywords should show help

-- 
gitgitgadget