Bazaar

Merge lp:~lifeless/bzr/commit into lp:bzr

  1. commit
  2. Merge into bzr.dev
Proposed by Robert Collins
Status: Merged
Approved by: John A Meinel
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~lifeless/bzr/commit
Merge into: lp:bzr
Diff against target: 158 lines (+57/-22)
4 files modified
NEWS (+4/-0)
bzrlib/builtins.py (+20/-16)
bzrlib/msgeditor.py (+17/-4)
bzrlib/tests/blackbox/test_commit.py (+16/-2)
To merge this branch: bzr merge lp:~lifeless/bzr/commit
Reviewer Review Type Date Requested Status
John A Meinel Needs Fixing
Review via email: mp+22920@code.launchpad.net

Commit message

(robertc) bzr commit will prompt before using a commit message that was generated by a template and not edited by the user. (Robert Collins)

Description of the change

This branch slightly cleans up the cmd layer of commit, which I did while figuring out the best way to make the change. The functional change is to add a prompt before accepting an unaltered template file.

To post a comment you must log in.
Revision history for this message
John A Meinel (jameinel) wrote :

You don't seem to have a test case for the 'failing' side. Where the user is prompted but they request to *not* commit.

That code path seems to:
  return ""

Whereas the other code paths when the template file doesn't exist, or _run_editor fails:
  return None

Otherwise it seems ok.

review: Needs Fixing

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'NEWS'
--- NEWS 2010-04-06 11:29:06 +0000
+++ NEWS 2010-04-07 22:25:45 +0000
@@ -29,6 +29,10 @@
29Improvements29Improvements
30************30************
3131
32* ``bzr commit`` will prompt before using a commit message that was
33 generated by a template and not edited by the user.
34 (Robert Collins, #530265)
35
32* Less code is loaded at startup. (Cold-cache start time is about 10-20%36* Less code is loaded at startup. (Cold-cache start time is about 10-20%
33 less.)37 less.)
34 (Martin Pool, #553017)38 (Martin Pool, #553017)
3539
=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py 2010-04-06 21:46:47 +0000
+++ bzrlib/builtins.py 2010-04-07 22:25:45 +0000
@@ -3150,31 +3150,37 @@
3150 '(use --file "%(f)s" to take commit message from that file)'3150 '(use --file "%(f)s" to take commit message from that file)'
3151 % { 'f': message })3151 % { 'f': message })
3152 ui.ui_factory.show_warning(warning_msg)3152 ui.ui_factory.show_warning(warning_msg)
3153 if '\r' in message:
3154 message = message.replace('\r\n', '\n')
3155 message = message.replace('\r', '\n')
3156 if file:
3157 raise errors.BzrCommandError(
3158 "please specify either --message or --file")
31533159
3154 def get_message(commit_obj):3160 def get_message(commit_obj):
3155 """Callback to get commit message"""3161 """Callback to get commit message"""
3156 my_message = message3162 if file:
3157 if my_message is not None and '\r' in my_message:3163 my_message = codecs.open(
3158 my_message = my_message.replace('\r\n', '\n')3164 file, 'rt', osutils.get_user_encoding()).read()
3159 my_message = my_message.replace('\r', '\n')3165 elif message is not None:
3160 if my_message is None and not file:3166 my_message = message
3161 # t is the status of the tree3167 else:
3162 t = make_commit_message_template_encoded(tree,3168 # No message supplied: make one up.
3169 # text is the status of the tree
3170 text = make_commit_message_template_encoded(tree,
3163 selected_list, diff=show_diff,3171 selected_list, diff=show_diff,
3164 output_encoding=osutils.get_user_encoding())3172 output_encoding=osutils.get_user_encoding())
3165 # start_message is the template generated from hooks3173 # start_message is the template generated from hooks
3174 # XXX: Warning - looks like hooks return unicode,
3175 # make_commit_message_template_encoded returns user encoding.
3176 # We probably want to be using edit_commit_message instead to
3177 # avoid this.
3166 start_message = generate_commit_message_template(commit_obj)3178 start_message = generate_commit_message_template(commit_obj)
3167 my_message = edit_commit_message_encoded(t,3179 my_message = edit_commit_message_encoded(text,
3168 start_message=start_message)3180 start_message=start_message)
3169 if my_message is None:3181 if my_message is None:
3170 raise errors.BzrCommandError("please specify a commit"3182 raise errors.BzrCommandError("please specify a commit"
3171 " message with either --message or --file")3183 " message with either --message or --file")
3172 elif my_message and file:
3173 raise errors.BzrCommandError(
3174 "please specify either --message or --file")
3175 if file:
3176 my_message = codecs.open(file, 'rt',
3177 osutils.get_user_encoding()).read()
3178 if my_message == "":3184 if my_message == "":
3179 raise errors.BzrCommandError("empty commit message specified")3185 raise errors.BzrCommandError("empty commit message specified")
3180 return my_message3186 return my_message
@@ -3192,8 +3198,6 @@
3192 timezone=offset,3198 timezone=offset,
3193 exclude=safe_relpath_files(tree, exclude))3199 exclude=safe_relpath_files(tree, exclude))
3194 except PointlessCommit:3200 except PointlessCommit:
3195 # FIXME: This should really happen before the file is read in;
3196 # perhaps prepare the commit; get the message; then actually commit
3197 raise errors.BzrCommandError("No changes to commit."3201 raise errors.BzrCommandError("No changes to commit."
3198 " Use --unchanged to commit anyhow.")3202 " Use --unchanged to commit anyhow.")
3199 except ConflictsInTree:3203 except ConflictsInTree:
32003204
=== modified file 'bzrlib/msgeditor.py'
--- bzrlib/msgeditor.py 2010-02-17 17:11:16 +0000
+++ bzrlib/msgeditor.py 2010-04-07 22:25:45 +0000
@@ -27,6 +27,8 @@
27 config,27 config,
28 osutils,28 osutils,
29 trace,29 trace,
30 transport,
31 ui,
30 )32 )
31from bzrlib.errors import BzrError, BadCommitMessageEncoding33from bzrlib.errors import BzrError, BadCommitMessageEncoding
32from bzrlib.hooks import HookPoint, Hooks34from bzrlib.hooks import HookPoint, Hooks
@@ -139,10 +141,21 @@
139 try:141 try:
140 msgfilename, hasinfo = _create_temp_file_with_commit_template(142 msgfilename, hasinfo = _create_temp_file_with_commit_template(
141 infotext, ignoreline, start_message)143 infotext, ignoreline, start_message)
142144 if not msgfilename:
143 if not msgfilename or not _run_editor(msgfilename):145 return None
144 return None146 basename = osutils.basename(msgfilename)
145147 msg_transport = transport.get_transport(osutils.dirname(msgfilename))
148 reference_content = msg_transport.get_bytes(basename)
149 if not _run_editor(msgfilename):
150 return None
151 edited_content = msg_transport.get_bytes(basename)
152 if edited_content == reference_content:
153 if not ui.ui_factory.get_boolean(
154 "Commit message was not edited, use anyway"):
155 # Returning "" makes cmd_commit raise 'empty commit message
156 # specified' which is a reasonable error, given the user has
157 # rejected using the unedited template.
158 return ""
146 started = False159 started = False
147 msg = []160 msg = []
148 lastline, nlines = 0, 0161 lastline, nlines = 0, 0
149162
=== modified file 'bzrlib/tests/blackbox/test_commit.py'
--- bzrlib/tests/blackbox/test_commit.py 2010-02-23 07:43:11 +0000
+++ bzrlib/tests/blackbox/test_commit.py 2010-04-07 22:25:45 +0000
@@ -675,7 +675,7 @@
675 self.assertContainsRe(err,675 self.assertContainsRe(err,
676 r'^bzr: ERROR: Cannot lock.*readonly transport')676 r'^bzr: ERROR: Cannot lock.*readonly transport')
677677
678 def test_commit_hook_template(self):678 def setup_editor(self):
679 # Test that commit template hooks work679 # Test that commit template hooks work
680 if sys.platform == "win32":680 if sys.platform == "win32":
681 f = file('fed.bat', 'w')681 f = file('fed.bat', 'w')
@@ -688,11 +688,25 @@
688 f.close()688 f.close()
689 os.chmod('fed.sh', 0755)689 os.chmod('fed.sh', 0755)
690 osutils.set_or_unset_env('BZR_EDITOR', "./fed.sh")690 osutils.set_or_unset_env('BZR_EDITOR', "./fed.sh")
691
692 def setup_commit_with_template(self):
693 self.setup_editor()
691 msgeditor.hooks.install_named_hook("commit_message_template",694 msgeditor.hooks.install_named_hook("commit_message_template",
692 lambda commit_obj, msg: "save me some typing\n", None)695 lambda commit_obj, msg: "save me some typing\n", None)
693 tree = self.make_branch_and_tree('tree')696 tree = self.make_branch_and_tree('tree')
694 self.build_tree(['tree/hello.txt'])697 self.build_tree(['tree/hello.txt'])
695 tree.add('hello.txt')698 tree.add('hello.txt')
696 out, err = self.run_bzr("commit tree/hello.txt")699 return tree
700
701 def test_commit_hook_template_accepted(self):
702 tree = self.setup_commit_with_template()
703 out, err = self.run_bzr("commit tree/hello.txt", stdin="y\n")
697 last_rev = tree.branch.repository.get_revision(tree.last_revision())704 last_rev = tree.branch.repository.get_revision(tree.last_revision())
698 self.assertEqual('save me some typing\n', last_rev.message)705 self.assertEqual('save me some typing\n', last_rev.message)
706
707 def test_commit_hook_template_rejected(self):
708 tree = self.setup_commit_with_template()
709 expected = tree.last_revision()
710 out, err = self.run_bzr_error(["empty commit message"],
711 "commit tree/hello.txt", stdin="n\n")
712 self.assertEqual(expected, tree.last_revision())