'stg edit' aborts on empty patch description; add instructions.

This resolves #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 #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.

Author: topher200

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.
 

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(

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')
 

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:

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 <<EOF
+echo "" > "\$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 <<EOF
 sed 's/Empty patch/Empty Patch/' "\$1" > "\$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 <[email protected]>\nDate: <omitted>'
-  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: <omitted>/" ; }
 
 test_expect_success 'Save template to file' '
@@ -121,45 +135,45 @@ 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
 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
 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
 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