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
1=== modified file 'NEWS'
2--- NEWS 2010-04-06 11:29:06 +0000
3+++ NEWS 2010-04-07 22:25:45 +0000
4@@ -29,6 +29,10 @@
5 Improvements
6 ************
7
8+* ``bzr commit`` will prompt before using a commit message that was
9+ generated by a template and not edited by the user.
10+ (Robert Collins, #530265)
11+
12 * Less code is loaded at startup. (Cold-cache start time is about 10-20%
13 less.)
14 (Martin Pool, #553017)
15
16=== modified file 'bzrlib/builtins.py'
17--- bzrlib/builtins.py 2010-04-06 21:46:47 +0000
18+++ bzrlib/builtins.py 2010-04-07 22:25:45 +0000
19@@ -3150,31 +3150,37 @@
20 '(use --file "%(f)s" to take commit message from that file)'
21 % { 'f': message })
22 ui.ui_factory.show_warning(warning_msg)
23+ if '\r' in message:
24+ message = message.replace('\r\n', '\n')
25+ message = message.replace('\r', '\n')
26+ if file:
27+ raise errors.BzrCommandError(
28+ "please specify either --message or --file")
29
30 def get_message(commit_obj):
31 """Callback to get commit message"""
32- my_message = message
33- if my_message is not None and '\r' in my_message:
34- my_message = my_message.replace('\r\n', '\n')
35- my_message = my_message.replace('\r', '\n')
36- if my_message is None and not file:
37- # t is the status of the tree
38- t = make_commit_message_template_encoded(tree,
39+ if file:
40+ my_message = codecs.open(
41+ file, 'rt', osutils.get_user_encoding()).read()
42+ elif message is not None:
43+ my_message = message
44+ else:
45+ # No message supplied: make one up.
46+ # text is the status of the tree
47+ text = make_commit_message_template_encoded(tree,
48 selected_list, diff=show_diff,
49 output_encoding=osutils.get_user_encoding())
50 # start_message is the template generated from hooks
51+ # XXX: Warning - looks like hooks return unicode,
52+ # make_commit_message_template_encoded returns user encoding.
53+ # We probably want to be using edit_commit_message instead to
54+ # avoid this.
55 start_message = generate_commit_message_template(commit_obj)
56- my_message = edit_commit_message_encoded(t,
57+ my_message = edit_commit_message_encoded(text,
58 start_message=start_message)
59 if my_message is None:
60 raise errors.BzrCommandError("please specify a commit"
61 " message with either --message or --file")
62- elif my_message and file:
63- raise errors.BzrCommandError(
64- "please specify either --message or --file")
65- if file:
66- my_message = codecs.open(file, 'rt',
67- osutils.get_user_encoding()).read()
68 if my_message == "":
69 raise errors.BzrCommandError("empty commit message specified")
70 return my_message
71@@ -3192,8 +3198,6 @@
72 timezone=offset,
73 exclude=safe_relpath_files(tree, exclude))
74 except PointlessCommit:
75- # FIXME: This should really happen before the file is read in;
76- # perhaps prepare the commit; get the message; then actually commit
77 raise errors.BzrCommandError("No changes to commit."
78 " Use --unchanged to commit anyhow.")
79 except ConflictsInTree:
80
81=== modified file 'bzrlib/msgeditor.py'
82--- bzrlib/msgeditor.py 2010-02-17 17:11:16 +0000
83+++ bzrlib/msgeditor.py 2010-04-07 22:25:45 +0000
84@@ -27,6 +27,8 @@
85 config,
86 osutils,
87 trace,
88+ transport,
89+ ui,
90 )
91 from bzrlib.errors import BzrError, BadCommitMessageEncoding
92 from bzrlib.hooks import HookPoint, Hooks
93@@ -139,10 +141,21 @@
94 try:
95 msgfilename, hasinfo = _create_temp_file_with_commit_template(
96 infotext, ignoreline, start_message)
97-
98- if not msgfilename or not _run_editor(msgfilename):
99- return None
100-
101+ if not msgfilename:
102+ return None
103+ basename = osutils.basename(msgfilename)
104+ msg_transport = transport.get_transport(osutils.dirname(msgfilename))
105+ reference_content = msg_transport.get_bytes(basename)
106+ if not _run_editor(msgfilename):
107+ return None
108+ edited_content = msg_transport.get_bytes(basename)
109+ if edited_content == reference_content:
110+ if not ui.ui_factory.get_boolean(
111+ "Commit message was not edited, use anyway"):
112+ # Returning "" makes cmd_commit raise 'empty commit message
113+ # specified' which is a reasonable error, given the user has
114+ # rejected using the unedited template.
115+ return ""
116 started = False
117 msg = []
118 lastline, nlines = 0, 0
119
120=== modified file 'bzrlib/tests/blackbox/test_commit.py'
121--- bzrlib/tests/blackbox/test_commit.py 2010-02-23 07:43:11 +0000
122+++ bzrlib/tests/blackbox/test_commit.py 2010-04-07 22:25:45 +0000
123@@ -675,7 +675,7 @@
124 self.assertContainsRe(err,
125 r'^bzr: ERROR: Cannot lock.*readonly transport')
126
127- def test_commit_hook_template(self):
128+ def setup_editor(self):
129 # Test that commit template hooks work
130 if sys.platform == "win32":
131 f = file('fed.bat', 'w')
132@@ -688,11 +688,25 @@
133 f.close()
134 os.chmod('fed.sh', 0755)
135 osutils.set_or_unset_env('BZR_EDITOR', "./fed.sh")
136+
137+ def setup_commit_with_template(self):
138+ self.setup_editor()
139 msgeditor.hooks.install_named_hook("commit_message_template",
140 lambda commit_obj, msg: "save me some typing\n", None)
141 tree = self.make_branch_and_tree('tree')
142 self.build_tree(['tree/hello.txt'])
143 tree.add('hello.txt')
144- out, err = self.run_bzr("commit tree/hello.txt")
145+ return tree
146+
147+ def test_commit_hook_template_accepted(self):
148+ tree = self.setup_commit_with_template()
149+ out, err = self.run_bzr("commit tree/hello.txt", stdin="y\n")
150 last_rev = tree.branch.repository.get_revision(tree.last_revision())
151 self.assertEqual('save me some typing\n', last_rev.message)
152+
153+ def test_commit_hook_template_rejected(self):
154+ tree = self.setup_commit_with_template()
155+ expected = tree.last_revision()
156+ out, err = self.run_bzr_error(["empty commit message"],
157+ "commit tree/hello.txt", stdin="n\n")
158+ self.assertEqual(expected, tree.last_revision())