From ad0d8d3a4be54ca836110756c7f632b7e6bf7ef0 Mon Sep 17 00:00:00 2001 From: Topher Brown Date: Sun, 18 Jul 2021 00:44:06 -0400 Subject: [PATCH] 'stg edit' aborts on empty patch description; add instructions. This resolves https://github.com/stacked-git/stgit/issues/138 by... 1. Adding instruction hints on the *power* of 'stg edit'. 2. Stripping these new hint lines from the description. 3. Aborting 'stg edit' if all description lines are cleared. From conversation on https://github.com/stacked-git/stgit/issues/138 it became clear that aborting the 'edit' on an empty editor is correct. This behavior mirrors 'git commit' and 'stg squash'. Previously, if a user cleared the 'stg edit' editor window, the patch description would be cleared. This is unintuative! Users likely expect that this would abort the edit instead. --- stgit/commands/common.py | 16 +++++++++++++--- stgit/commands/imprt.py | 2 +- stgit/commands/squash.py | 9 ++------- stgit/lib/edit.py | 9 ++++++++- t/t3300-edit.sh | 30 ++++++++++++++++++++++-------- 5 files changed, 46 insertions(+), 20 deletions(-) diff --git a/stgit/commands/common.py b/stgit/commands/common.py index 855f7a1d..7b44d3c6 100644 --- a/stgit/commands/common.py +++ b/stgit/commands/common.py @@ -369,11 +369,15 @@ def __split_descr_diff(string): return desc, diff -def __parse_description(descr): +def __parse_description(descr, fail_on_empty_description): """Parse the patch description for patch and author information.""" subject = '' patch_name = authname = authemail = authdate = None + descr = '\n'.join(strip_comments(descr)).strip() + if fail_on_empty_description and not descr: + raise CmdException('Aborting edit due to empty patch description') + descr_lines = [line.rstrip() for line in descr.splitlines()] lasthdr = 0 @@ -415,7 +419,7 @@ def __parse_description(descr): return (message, patch_name, authname, authemail, authdate) -def parse_patch(patch_data, contains_diff): +def parse_patch(patch_data, contains_diff, fail_on_empty_description=True): """Parse patch data. Returns (description, patch_name, authname, authemail, authdate, diff) @@ -428,12 +432,18 @@ def parse_patch(patch_data, contains_diff): descr = patch_data diff = None (descr, patch_name, authname, authemail, authdate) = __parse_description( - decode_utf8_with_latin1(descr) + decode_utf8_with_latin1(descr), fail_on_empty_description ) return (descr, patch_name, authname, authemail, authdate, diff) +def strip_comments(message): + for line in message.splitlines(): + if not line.startswith('#'): + yield line + + def run_commit_msg_hook(repo, cd, editor_is_used=True): """Run the commit-msg hook (if any) on a commit. diff --git a/stgit/commands/imprt.py b/stgit/commands/imprt.py index ef8af076..c15723a1 100644 --- a/stgit/commands/imprt.py +++ b/stgit/commands/imprt.py @@ -286,7 +286,7 @@ def __import_file(filename, options): f.close() message, patch_name, author_name, author_email, author_date, diff = parse_patch( - patch_data, contains_diff=True + patch_data, contains_diff=True, fail_on_empty_description=False ) __create_patch( diff --git a/stgit/commands/squash.py b/stgit/commands/squash.py index 5018bed6..87016693 100644 --- a/stgit/commands/squash.py +++ b/stgit/commands/squash.py @@ -5,6 +5,7 @@ DirectoryHasRepository, parse_patches, run_commit_msg_hook, + strip_comments, ) from stgit.lib.transaction import StackTransaction, TransactionHalted @@ -61,12 +62,6 @@ class SaveTemplateDone(Exception): pass -def _strip_comments(message): - for line in message.splitlines(): - if not line.startswith('#'): - yield line - - def _squash_patches(trans, patches, name, msg, save_template, no_verify=False): cd = trans.patches[patches[0]].data for pn in patches[1:]: @@ -101,7 +96,7 @@ def _squash_patches(trans, patches, name, msg, save_template, no_verify=False): else: msg = utils.edit_string(msg, '.stgit-squash.txt') - msg = '\n'.join(_strip_comments(msg)).strip() + msg = '\n'.join(strip_comments(msg)).strip() if not msg: raise CmdException('Aborting squash due to empty commit message') diff --git a/stgit/lib/edit.py b/stgit/lib/edit.py index ee1cbd6a..af6f60bb 100644 --- a/stgit/lib/edit.py +++ b/stgit/lib/edit.py @@ -11,6 +11,12 @@ from stgit.lib.log import log_stack_state from stgit.out import out +EDIT_MESSAGE_INSTRUCTIONS = """# Everything here is editable! You can modify the patch name, author, +# date, commit message, and the diff (if --diff was given). +# Lines starting with '#' will be ignored, and an empty message +# aborts the edit. +""" + def update_patch_description(repo, cd, text, contains_diff): """Create commit with updated description. @@ -68,10 +74,11 @@ def patch_desc(repo, cd, patch_name, append_diff, diff_flags, replacement_diff): 'Date: %s' % cd.author.date.isoformat(), '', cd.message_str, + EDIT_MESSAGE_INSTRUCTIONS, ] ).encode(commit_encoding) if append_diff: - parts = [desc.rstrip(), b'', b'---', b''] + parts = [desc.rstrip(), b'---', b''] if replacement_diff: parts.append(replacement_diff) else: diff --git a/t/t3300-edit.sh b/t/t3300-edit.sh index 21b73801..3e6026b5 100755 --- a/t/t3300-edit.sh +++ b/t/t3300-edit.sh @@ -31,6 +31,15 @@ msg () { git cat-file -p $1 | sed '1,/^$/d' | tr '\n' / | sed 's,/*$,,' ; } auth () { git log -n 1 --pretty=format:"%an, %ae" $1 ; } adate () { git log -n 1 --pretty=format:%ai $1 ; } +write_script diffedit < "\$1" +EOF +test_expect_success 'Empty editor aborts edit' ' + EDITOR=./diffedit command_error stg edit 2>&1 | + grep -e "Aborting edit due to empty patch description" +' +rm -f diffedit + write_script diffedit < "\$1".tmp && mv "\$1".tmp "\$1" EOF @@ -84,7 +93,12 @@ test_expect_success 'Set patch message with --file -' ' ' ( printf 'Patch: p2\nFrom: A Ú Thor \nDate: ' - printf '\n\nPride and Prejudice\n' ) > expected-tmpl + printf '\n\nPride and Prejudice\n' + printf '\n# Everything here is editable! You can modify the patch name, author,' + printf '\n# date, commit message, and the diff (if --diff was given).' + printf "\n# Lines starting with '#' will be ignored, and an empty message" + printf '\n# aborts the edit.\n' +) > expected-tmpl omit_date () { sed "s/^Date:.*$/Date: /" ; } test_expect_success 'Save template to file' ' @@ -121,7 +135,7 @@ test_expect_success 'Edit commit message interactively (vi)' ' unset EDITOR m=$(msg HEAD) && PATH=.:$PATH stg edit p2 && - test "$(msg HEAD)" = "$m/vi" + test "$(msg HEAD)" = "$m//vi" ' mkeditor e1 @@ -129,14 +143,14 @@ test_expect_success 'Edit commit message interactively (EDITOR)' ' m=$(msg HEAD) && EDITOR=./e1 PATH=.:$PATH stg edit p2 && echo $m && echo $(msg HEAD) && - test "$(msg HEAD)" = "$m/e1" + test "$(msg HEAD)" = "$m//e1" ' mkeditor e2 test_expect_success 'Edit commit message interactively (VISUAL)' ' m=$(msg HEAD) && VISUAL=./e2 EDITOR=./e1 PATH=.:$PATH stg edit p2 && - test "$(msg HEAD)" = "$m/e2" + test "$(msg HEAD)" = "$m//e2" ' mkeditor e3 @@ -144,7 +158,7 @@ test_expect_success 'Edit commit message interactively (core.editor)' ' m=$(msg HEAD) && git config core.editor e3 && VISUAL=./e2 EDITOR=./e1 PATH=.:$PATH stg edit p2 && - test "$(msg HEAD)" = "$m/e3" + test "$(msg HEAD)" = "$m//e3" ' mkeditor e4 @@ -152,14 +166,14 @@ test_expect_success 'Edit commit message interactively (stgit.editor)' ' m=$(msg HEAD) && git config stgit.editor e4 && VISUAL=./e2 EDITOR=./e1 PATH=.:$PATH stg edit p2 && - test "$(msg HEAD)" = "$m/e4" + test "$(msg HEAD)" = "$m//e4" ' mkeditor e5 test_expect_success 'Edit commit message interactively (GIT_EDITOR)' ' m=$(msg HEAD) && GIT_EDITOR=./e5 VISUAL=./e2 EDITOR=./e1 PATH=.:$PATH stg edit p2 && - test "$(msg HEAD)" = "$m/e5" + test "$(msg HEAD)" = "$m//e5" ' rm -f vi e1 e2 e3 e4 e5 @@ -169,7 +183,7 @@ git config --unset stgit.editor mkeditor twoliner test_expect_success 'Both noninterative and interactive editing' ' EDITOR=./twoliner stg edit -e -m "oneliner" p2 && - test "$(msg HEAD)" = "oneliner/twoliner" + test "$(msg HEAD)" = "oneliner//twoliner" ' rm -f twoliner