git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Ben Keene via GitGitGadget <gitgitgadget@gmail.com>
Date
Dec 13, 2019, 13:57 UTC
Message-ID
<pull.675.v4.git.git.1576245481.gitgitgadget@gmail.com>
In-Reply-To
<pull.675.v3.git.git.1576179987.gitgitgadget@gmail.com>

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) 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 2 displays the context help for the failed command.
3) If Git generates an error while attempting to clean up the RCS Keyword
expansions, it currently leaves P4 in an invalid state. Files that were
checked out by P4 are not revereted.

Commit 3 adds and exception handler that catches this condition and issues a P4 Revert for the files that were previously edited.

4) 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 4 adds a helpful suggestion, that the user might want to set git-p4.attemptRCSCleanup.

Revisions =========

v3 - Implemented the various suggestions from Luke and Denton.

I did not add additional exception handling for the EOFError in the prompt method. I do believe that it is a good idea, but that would change the logic handling of the existing code to handle this new "no answer" condition and I didn't want to introduce that at this time.

v4 - Whitespace clean up and commit clarifications.

Submit 3 suggested some clarifications to the commit test and revealed some whitespace errors.

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 | 93 +++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 59 insertions(+), 34 deletions(-)
base-commit: ad05a3d8e5a6a06443836b5e40434262d992889a
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-675%2Fseraphire%2Fseraphire%2Fp4-usability-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-675/seraphire/seraphire/p4-usability-v4
Pull-Request: https://github.com/git/git/pull/675
Range-diff vs v3:
 1:  fff93acf44 ! 1:  6c23cd5684 git-p4: yes/no prompts should sanitize user text
     @@ -10,10 +10,10 @@
          will fail.
      
          Create a new function, prompt(prompt_text) where
     -      * promt_text is the text prompt for the user
     -      * choices are extracted from the prompt text [.]
     -          a single letter surrounded by square brackets
     -          is selected as a valid choice.
     +      * prompt_text is the text prompt for the user
     +      * returns a single character where valid return values are
     +          found by inspecting prompt_text for single characters
     +          surrounded by square brackets
      
          This new function must  prompt the user for input and sanitize it by
          converting the response to a lower case string, trimming leading and
     @@ -26,6 +26,7 @@
          choices, remove the loop from the calling code that handles response
          verification.
      
     +    Thanks-to: Denton Liu <Denton Liu>
          Signed-off-by: Ben Keene <seraphire@gmail.com>
      
       diff --git a/git-p4.py b/git-p4.py
     @@ -119,8 +120,3 @@
                               break
       
               chdir(self.oldWorkingDirectory)
     -@@
     - 
     - if __name__ == '__main__':
     -     main()
     -+
 2:  5c5c981632 = 2:  bfdd3dc517 git-p4: show detailed help when parsing options fail
 3:  c466e79148 = 3:  20f6398693 git-p4: wrap patchRCSKeywords test to revert changes on failure
 4:  00307c3951 = 4:  c78e2e4db1 git-p4: failure because of RCS keywords should show help
-- 
gitgitgadget
Previous: Ben KeeneNext: Ben Keene via GitGitGadget
Message 30 of 46 in “git-p4: Usability enhancements”
  1. 0/3 git-p4: Usability enhancementsBen Keene via GitGitGadget, Dec 9, 2019
  2. 1/3 git-p4: [usability] yes/no prompts should sanitize user textBen Keene via GitGitGadget, Dec 9, 2019
  3. Junio C HamanoDec 9, 2019
  4. Ben KeeneDec 10, 2019
  5. 2/3 git-p4: [usability] RCS Keyword failure should suggest helpBen Keene via GitGitGadget, Dec 9, 2019
  6. Junio C HamanoDec 9, 2019
  7. 3/3 git-p4: [usability] Show detailed help when parsing options failBen Keene via GitGitGadget, Dec 9, 2019
  8. Junio C HamanoDec 9, 2019
  9. Junio C HamanoDec 9, 2019
  10. 0/4 git-p4: Usability enhancementsBen Keene via GitGitGadget, Dec 10, 2019
  11. 2/4 git-p4: show detailed help when parsing options failBen Keene via GitGitGadget, Dec 10, 2019
  12. 4/4 git-p4: failure because of RCS keywords should show helpBen Keene via GitGitGadget, Dec 10, 2019
  13. Denton LiuDec 11, 2019
  14. 3/4 git-p4: wrap patchRCSKeywords test to revert changes on failureBen Keene via GitGitGadget, Dec 10, 2019
  15. 1/4 git-p4: yes/no prompts should sanitize user textBen Keene via GitGitGadget, Dec 10, 2019
  16. Denton LiuDec 11, 2019
  17. Denton LiuDec 11, 2019
  18. Luke DiamandDec 11, 2019
  19. 0/4 git-p4: Usability enhancementsBen Keene via GitGitGadget, Dec 12, 2019
  20. 2/4 git-p4: show detailed help when parsing options failBen Keene via GitGitGadget, Dec 12, 2019
  21. 3/4 git-p4: wrap patchRCSKeywords test to revert changes on failureBen Keene via GitGitGadget, Dec 12, 2019
  22. 4/4 git-p4: failure because of RCS keywords should show helpBen Keene via GitGitGadget, Dec 12, 2019
  23. 1/4 git-p4: yes/no prompts should sanitize user textBen Keene via GitGitGadget, Dec 12, 2019
  24. Denton LiuDec 13, 2019
  25. Ben KeeneDec 13, 2019
  26. Junio C HamanoDec 13, 2019
  27. Johannes SchindelinDec 15, 2019
  28. Junio C HamanoDec 16, 2019
  29. Ben KeeneDec 16, 2019
  30. 0/4 git-p4: Usability enhancementsBen Keene via GitGitGadget, Dec 13, 2019
  31. 2/4 git-p4: show detailed help when parsing options failBen Keene via GitGitGadget, Dec 13, 2019
  32. 3/4 git-p4: wrap patchRCSKeywords test to revert changes on failureBen Keene via GitGitGadget, Dec 13, 2019
  33. 1/4 git-p4: yes/no prompts should sanitize user textBen Keene via GitGitGadget, Dec 13, 2019
  34. Denton LiuDec 13, 2019
  35. Ben KeeneDec 16, 2019
  36. 4/4 git-p4: failure because of RCS keywords should show helpBen Keene via GitGitGadget, Dec 13, 2019
  37. 0/4 git-p4: Usability enhancementsBen Keene via GitGitGadget, Dec 16, 2019
  38. 1/4 git-p4: yes/no prompts should sanitize user textBen Keene via GitGitGadget, Dec 16, 2019
  39. 4/4 git-p4: failure because of RCS keywords should show helpBen Keene via GitGitGadget, Dec 16, 2019
  40. 3/4 git-p4: wrap patchRCSKeywords test to revert changes on failureBen Keene via GitGitGadget, Dec 16, 2019
  41. 2/4 git-p4: show detailed help when parsing options failBen Keene via GitGitGadget, Dec 16, 2019
  42. Junio C HamanoDec 16, 2019
  43. Luke DiamandDec 21, 2019
  44. Junio C HamanoDec 25, 2019
  45. Ben KeeneJan 2, 2020
  46. Junio C HamanoJan 2, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.