git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Ben Keene via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Ben Keene <seraphire@gmail.com>, Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2 0/4] git-p4: Usability enhancements
Date: Tue, 10 Dec 2019 15:22:50 +0000	[thread overview]
Message-ID: <pull.675.v2.git.git.1575991374.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.675.git.git.1575901009.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) 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 <seraphire@gmail.com>
      
     -    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 <seraphire@gmail.com>
      
     -    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 <seraphire@gmail.com>
      
     -    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 <seraphire@gmail.com>
      
     @@ -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

  parent reply	other threads:[~2019-12-10 15:23 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-09 14:16 [PATCH 0/3] git-p4: Usability enhancements Ben Keene via GitGitGadget
2019-12-09 14:16 ` [PATCH 1/3] git-p4: [usability] yes/no prompts should sanitize user text Ben Keene via GitGitGadget
2019-12-09 22:00   ` Junio C Hamano
2019-12-10 14:26     ` Ben Keene
2019-12-09 14:16 ` [PATCH 2/3] git-p4: [usability] RCS Keyword failure should suggest help Ben Keene via GitGitGadget
2019-12-09 22:22   ` Junio C Hamano
2019-12-09 14:16 ` [PATCH 3/3] git-p4: [usability] Show detailed help when parsing options fail Ben Keene via GitGitGadget
2019-12-09 22:24   ` Junio C Hamano
2019-12-09 22:06 ` [PATCH 0/3] git-p4: Usability enhancements Junio C Hamano
2019-12-10 15:22 ` Ben Keene via GitGitGadget [this message]
2019-12-10 15:22   ` [PATCH v2 1/4] git-p4: yes/no prompts should sanitize user text Ben Keene via GitGitGadget
2019-12-11 11:52     ` Denton Liu
2019-12-11 11:59     ` Denton Liu
2019-12-10 15:22   ` [PATCH v2 2/4] git-p4: show detailed help when parsing options fail Ben Keene via GitGitGadget
2019-12-10 15:22   ` [PATCH v2 3/4] git-p4: wrap patchRCSKeywords test to revert changes on failure Ben Keene via GitGitGadget
2019-12-10 15:22   ` [PATCH v2 4/4] git-p4: failure because of RCS keywords should show help Ben Keene via GitGitGadget
2019-12-11 11:29     ` Denton Liu
2019-12-11  9:43   ` [PATCH v2 0/4] git-p4: Usability enhancements Luke Diamand
2019-12-12 19:46   ` [PATCH v3 " Ben Keene via GitGitGadget
2019-12-12 19:46     ` [PATCH v3 1/4] git-p4: yes/no prompts should sanitize user text Ben Keene via GitGitGadget
2019-12-13  1:45       ` Denton Liu
2019-12-13 13:42         ` Ben Keene
2019-12-13 19:46         ` Junio C Hamano
2019-12-15 20:30           ` Johannes Schindelin
2019-12-16 17:54             ` Junio C Hamano
2019-12-16 19:11             ` Ben Keene
2019-12-12 19:46     ` [PATCH v3 2/4] git-p4: show detailed help when parsing options fail Ben Keene via GitGitGadget
2019-12-12 19:46     ` [PATCH v3 3/4] git-p4: wrap patchRCSKeywords test to revert changes on failure Ben Keene via GitGitGadget
2019-12-12 19:46     ` [PATCH v3 4/4] git-p4: failure because of RCS keywords should show help Ben Keene via GitGitGadget
2019-12-13 13:57     ` [PATCH v4 0/4] git-p4: Usability enhancements Ben Keene via GitGitGadget
2019-12-13 13:57       ` [PATCH v4 1/4] git-p4: yes/no prompts should sanitize user text Ben Keene via GitGitGadget
2019-12-13 22:54         ` Denton Liu
2019-12-16 13:53           ` Ben Keene
2019-12-13 13:57       ` [PATCH v4 2/4] git-p4: show detailed help when parsing options fail Ben Keene via GitGitGadget
2019-12-13 13:58       ` [PATCH v4 3/4] git-p4: wrap patchRCSKeywords test to revert changes on failure Ben Keene via GitGitGadget
2019-12-13 13:58       ` [PATCH v4 4/4] git-p4: failure because of RCS keywords should show help Ben Keene via GitGitGadget
2019-12-16 14:02       ` [PATCH v5 0/4] git-p4: Usability enhancements Ben Keene via GitGitGadget
2019-12-16 14:02         ` [PATCH v5 1/4] git-p4: yes/no prompts should sanitize user text Ben Keene via GitGitGadget
2019-12-16 14:02         ` [PATCH v5 2/4] git-p4: show detailed help when parsing options fail Ben Keene via GitGitGadget
2019-12-16 14:02         ` [PATCH v5 3/4] git-p4: wrap patchRCSKeywords test to revert changes on failure Ben Keene via GitGitGadget
2019-12-16 14:02         ` [PATCH v5 4/4] git-p4: failure because of RCS keywords should show help Ben Keene via GitGitGadget
2019-12-16 20:39         ` [PATCH v5 0/4] git-p4: Usability enhancements Junio C Hamano
2019-12-21 10:19           ` Luke Diamand
2019-12-25 19:13             ` Junio C Hamano
2020-01-02 13:50               ` Ben Keene
2020-01-02 21:44                 ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.675.v2.git.git.1575991374.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=seraphire@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).