Merge lp:~gz/bzr/resolve_file_not_done into lp:bzr

Proposed by Martin Packman on 2012-07-27
Status: Needs review
Proposed branch: lp:~gz/bzr/resolve_file_not_done
Merge into: lp:bzr
Prerequisite: lp:~gz/bzr/resolve_auto_refactor
Diff against target: 171 lines (+30/-21)
3 files modified
bzrlib/conflicts.py (+5/-7)
bzrlib/help_topics/en/conflict-types.txt (+4/-3)
bzrlib/tests/test_conflicts.py (+21/-11)
To merge this branch: bzr merge lp:~gz/bzr/resolve_file_not_done
Reviewer Review Type Date Requested Status
bzr-core 2012-07-27 Pending
Review via email: mp+117052@code.launchpad.net

Description of the change

Makes `bzr resolve FILE` use the same auto logic as `bzr resolve` rather than implying `bzr resolve --done FILE`.

This was discussed on the mailing list recently:

<https://lists.ubuntu.com/archives/bazaar/2012q3/075039.html>

Currently auto is pretty limited, which makes this change less appetizing, for deleted directories and such like I often end up running resolve with single argument quite often, and having to remember --done now could get frustrating. However, it's pretty obvious if you know about the change, and making auto smarter is something we've wanted for a while anyway.

I've updated some bits of documentation but am probably missing other bits elsewhere.

The current testing for conflicts I don't fully understand, some help explaining the details there would be good, but it appears that several places where I needed to add --done should get covered by making --auto smarter.

To post a comment you must log in.
Aaron Bentley (abentley) wrote :

I think that the existing behaviour makes sense, and the proposed changes just cause users to do more typing.

They encourage users to use "resolve FILE" when bzr doesn't need an argument, and they force users to specify --done when they actually do need to specify an argument.

Why have you removed documentation of --all at 19?

Martin Packman (gz) wrote :

> I think that the existing behaviour makes sense, and the proposed changes just
> cause users to do more typing.

I agree, the extra typing is bad.

My aim with this group of branches is to actually not change what you have to type. This branch makes it harder to just discard conflicts, in others I want to make detection of resolved conflicts actually work for more cases. So, in the end you'll still be doing some action to resolve a conflict, then `bzr resolve FILE`.. or just `bzr resolve`.

> Why have you removed documentation of --all at 19?

Should have left that for another branch really but, in short, `bzr resolve --all` is functionally equivalent to `bzr resolve --done`, which is a more dangerous operation than we want to recommend in the help text.

People should be using `bzr resolve` followed by `bzr resolve --done FILE` if the auto resolution fails for something in particular. Generalising to the no-arg form for all files is easy to understand from there for when it's actually useful.

Unmerged revisions

6549. By Martin Packman on 2012-07-27

Note in cmd_resolve help that --done is needed, and remove mention of --all

6548. By Martin Packman on 2012-07-27

Make 'auto' the default in resolve_action_registry

6547. By Martin Packman on 2012-07-27

Update conflict-types help for change to resolve semantics

6546. By Martin Packman on 2012-07-27

Make resolving with a list of files default to action auto rather than done

6545. By Martin Packman on 2012-07-27

Deprecate WorkingTree.auto_resolve

6544. By Martin Packman on 2012-07-27

Refactor auto_resolve tree method into auto action on conflicts

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'bzrlib/conflicts.py'
--- bzrlib/conflicts.py 2012-07-27 12:34:38 +0000
+++ bzrlib/conflicts.py 2012-07-27 12:34:38 +0000
@@ -92,7 +92,8 @@
92resolve_action_registry.register(92resolve_action_registry.register(
93 'take-other', 'take_other',93 'take-other', 'take_other',
94 'Resolve the conflict taking the merged version into account.')94 'Resolve the conflict taking the merged version into account.')
95resolve_action_registry.default_key = 'done'95resolve_action_registry.default_key = 'auto'
96
9697
97class ResolveActionOption(option.RegistryOption):98class ResolveActionOption(option.RegistryOption):
9899
@@ -112,8 +113,8 @@
112 before you can commit.113 before you can commit.
113114
114 Once you have fixed a problem, use "bzr resolve" to automatically mark115 Once you have fixed a problem, use "bzr resolve" to automatically mark
115 text conflicts as fixed, "bzr resolve FILE" to mark a specific conflict as116 text conflicts as fixed, "bzr resolve --done FILE" to mark a specific
116 resolved, or "bzr resolve --all" to mark all conflicts as resolved.117 conflict as resolved.
117 """118 """
118 aliases = ['resolved']119 aliases = ['resolved']
119 takes_args = ['file*']120 takes_args = ['file*']
@@ -137,10 +138,7 @@
137 tree, file_list = workingtree.WorkingTree.open_containing_paths(138 tree, file_list = workingtree.WorkingTree.open_containing_paths(
138 file_list, directory)139 file_list, directory)
139 if action is None:140 if action is None:
140 if file_list is None:141 action = 'auto'
141 action = 'auto'
142 else:
143 action = 'done'
144 before, after = resolve(tree, file_list, action=action)142 before, after = resolve(tree, file_list, action=action)
145 # GZ 2012-07-27: Should unify UI below now that auto is less magical.143 # GZ 2012-07-27: Should unify UI below now that auto is less magical.
146 if action == 'auto' and file_list is None:144 if action == 'auto' and file_list is None:
147145
=== modified file 'bzrlib/help_topics/en/conflict-types.txt'
--- bzrlib/help_topics/en/conflict-types.txt 2011-04-01 06:58:37 +0000
+++ bzrlib/help_topics/en/conflict-types.txt 2012-07-27 12:34:38 +0000
@@ -50,10 +50,11 @@
5050
51 ``bzr resolve FILE --action=done'51 ``bzr resolve FILE --action=done'
5252
53Note that this is the default action when a single file is involved so you can53Note that for some kinds of conflict, Bazaar can automatically detect that
54simply use::54they have been resolved. You will also be told about any remaining conflicts
55that still need resolving. Simply use::
5556
56 ``bzr resolve FILE``57 ``bzr resolve``
5758
58See ``bzr help resolve`` for more details.59See ``bzr help resolve`` for more details.
5960
6061
=== modified file 'bzrlib/tests/test_conflicts.py'
--- bzrlib/tests/test_conflicts.py 2012-02-23 23:41:51 +0000
+++ bzrlib/tests/test_conflicts.py 2012-07-27 12:34:38 +0000
@@ -755,16 +755,18 @@
755"""755"""
756756
757 def test_take_this(self):757 def test_take_this(self):
758 # GZ 2012-07-27: Using auto should learn how to resolve this?
758 self.run_script("""759 self.run_script("""
759$ bzr rm -q dir --force760$ bzr rm -q dir --force
760$ bzr resolve dir761$ bzr resolve --done dir
7612>2 conflicts resolved, 0 remaining7622>2 conflicts resolved, 0 remaining
762$ bzr commit -q --strict -m 'No more conflicts nor unknown files'763$ bzr commit -q --strict -m 'No more conflicts nor unknown files'
763""")764""")
764765
765 def test_take_other(self):766 def test_take_other(self):
767 # GZ 2012-07-27: What's this testing? Should remain conflicted?
766 self.run_script("""768 self.run_script("""
767$ bzr resolve dir769$ bzr resolve --done dir
7682>2 conflicts resolved, 0 remaining7702>2 conflicts resolved, 0 remaining
769$ bzr commit -q --strict -m 'No more conflicts nor unknown files'771$ bzr commit -q --strict -m 'No more conflicts nor unknown files'
770""")772""")
@@ -797,25 +799,28 @@
797"""799"""
798800
799 def test_keep_them_all(self):801 def test_keep_them_all(self):
802 # GZ 2012-07-27: What's this testing? Should remain conflicted?
800 self.run_script("""803 self.run_script("""
801$ bzr resolve dir804$ bzr resolve --done dir
8022>2 conflicts resolved, 0 remaining8052>2 conflicts resolved, 0 remaining
803$ bzr commit -q --strict -m 'No more conflicts nor unknown files'806$ bzr commit -q --strict -m 'No more conflicts nor unknown files'
804""")807""")
805808
806 def test_adopt_child(self):809 def test_adopt_child(self):
810 # GZ 2012-07-27: Using auto should learn how to resolve this?
807 self.run_script("""811 self.run_script("""
808$ bzr mv -q dir/file2 file2812$ bzr mv -q dir/file2 file2
809$ bzr rm -q dir --force813$ bzr rm -q dir --force
810$ bzr resolve dir814$ bzr resolve --done dir
8112>2 conflicts resolved, 0 remaining8152>2 conflicts resolved, 0 remaining
812$ bzr commit -q --strict -m 'No more conflicts nor unknown files'816$ bzr commit -q --strict -m 'No more conflicts nor unknown files'
813""")817""")
814818
815 def test_kill_them_all(self):819 def test_kill_them_all(self):
820 # GZ 2012-07-27: Using auto should learn how to resolve this?
816 self.run_script("""821 self.run_script("""
817$ bzr rm -q dir --force822$ bzr rm -q dir --force
818$ bzr resolve dir823$ bzr resolve --done dir
8192>2 conflicts resolved, 0 remaining8242>2 conflicts resolved, 0 remaining
820$ bzr commit -q --strict -m 'No more conflicts nor unknown files'825$ bzr commit -q --strict -m 'No more conflicts nor unknown files'
821""")826""")
@@ -861,25 +866,28 @@
861"""866"""
862867
863 def test_keep_them_all(self):868 def test_keep_them_all(self):
869 # GZ 2012-07-27: What's this testing? Should remain conflicted?
864 self.run_script("""870 self.run_script("""
865$ bzr resolve dir871$ bzr resolve --done dir
8662>2 conflicts resolved, 0 remaining8722>2 conflicts resolved, 0 remaining
867$ bzr commit -q --strict -m 'No more conflicts nor unknown files'873$ bzr commit -q --strict -m 'No more conflicts nor unknown files'
868""")874""")
869875
870 def test_adopt_child(self):876 def test_adopt_child(self):
877 # GZ 2012-07-27: Using auto should learn how to resolve this?
871 self.run_script("""878 self.run_script("""
872$ bzr mv -q dir/file2 file2879$ bzr mv -q dir/file2 file2
873$ bzr rm -q dir --force880$ bzr rm -q dir --force
874$ bzr resolve dir881$ bzr resolve --done dir
8752>2 conflicts resolved, 0 remaining8822>2 conflicts resolved, 0 remaining
876$ bzr commit -q --strict -m 'No more conflicts nor unknown files'883$ bzr commit -q --strict -m 'No more conflicts nor unknown files'
877""")884""")
878885
879 def test_kill_them_all(self):886 def test_kill_them_all(self):
887 # GZ 2012-07-27: Using auto should learn how to resolve this?
880 self.run_script("""888 self.run_script("""
881$ bzr rm -q dir --force889$ bzr rm -q dir --force
882$ bzr resolve dir890$ bzr resolve --done dir
8832>2 conflicts resolved, 0 remaining8912>2 conflicts resolved, 0 remaining
884$ bzr commit -q --strict -m 'No more conflicts nor unknown files'892$ bzr commit -q --strict -m 'No more conflicts nor unknown files'
885""")893""")
@@ -1020,21 +1028,23 @@
1020"""1028"""
10211029
1022 def test_take_this(self):1030 def test_take_this(self):
1031 # GZ 2012-07-27: What's this testing? Should auto understand it?
1023 self.run_script("""1032 self.run_script("""
1024$ bzr rm -q foo.new --force1033$ bzr rm -q foo.new --force
1025# FIXME: Isn't it weird that foo is now unkown even if foo.new has been put1034# FIXME: Isn't it weird that foo is now unknown even if foo.new has been put
1026# aside ? -- vila 0909161035# aside ? -- vila 090916
1027$ bzr add -q foo1036$ bzr add -q foo
1028$ bzr resolve foo.new1037$ bzr resolve --done foo.new
10292>1 conflict resolved, 0 remaining10382>1 conflict resolved, 0 remaining
1030$ bzr commit -q --strict -m 'No more conflicts nor unknown files'1039$ bzr commit -q --strict -m 'No more conflicts nor unknown files'
1031""")1040""")
10321041
1033 def test_take_other(self):1042 def test_take_other(self):
1043 # GZ 2012-07-27: Using auto should learn how to resolve this?
1034 self.run_script("""1044 self.run_script("""
1035$ bzr rm -q foo --force1045$ bzr rm -q foo --force
1036$ bzr mv -q foo.new foo1046$ bzr mv -q foo.new foo
1037$ bzr resolve foo1047$ bzr resolve --done foo
10382>1 conflict resolved, 0 remaining10482>1 conflict resolved, 0 remaining
1039$ bzr commit -q --strict -m 'No more conflicts nor unknown files'1049$ bzr commit -q --strict -m 'No more conflicts nor unknown files'
1040""")1050""")