Merge lp:~frankoid/bzr/status-shelve-message into lp:bzr

[Francis Devereux]

Make "bzr status" only output the "x shelves exist" message when no files are specified.

If you run a plain "bzr status" then you are asking bzr about the status of the whole tree, so it is reasonable for it to report shelves, but if you run "bzr status myfile" then you are just asking it about the status of myfile, so it shouldn't tell you about shelves. Also, this change will prevent the annoying Emacs warnings that happen when you visit a file inside a branch with shelves.

If a file is specified and that file is in a shelve then the shelves message is still not output. This behaviour is to keep "bzr status" fast and to keep the logic simple (there has been some discussion about this on the <email address hidden>).

[Jelmer Vernooij (jelmer)]

Hi Francis,

There seem to be two parts to this change:

1) passing the specified_files to all the post_status hooks
2) changing the behaviour of one of the post_status hooks, which reports shelve status

(1) seems like a good idea in any case.

Perhaps it would make sense to split this branch up into two. The hook changes should be able to land pretty quickly, the second (in whatever form) can land once we decide on the correct behaviour for status when a file is specified.

(personally, I would prefer to keep the existing behaviour of "bzr status FILE" reporting shelves, but let's keep that discussion on the mailing list - I'll follow up there)

[Francis Devereux (frankoid)]

Hi Jelmer,

I've separated (1) into lp:~frankoid/bzr/status-hook-params-specific-files

So far nobody on the mailing list has objected to "bzr status FILE" not reporting shelves as long as FILE isn't in a shelf, and most people seemed to want it to not report shelves in that case too, so it would be good to hear your thoughts on the mailing list.

Thanks,

Francis

[Martin Pool (mbp)]

I think this is an improvement as it is. I won't send it right away, so Jelmer can have his say if he wants to. Thanks, frankoid.

review: Approve

[Martin Pool (mbp)]

Francis, could you please do the Canonical contributor agreement at http://canonical.com/contributors

[Francis Devereux (frankoid)]

I think I signed the Canonical contributor agreement last week (on the 24th of November). Let me know if I need to try again.

[Martin Pool (mbp)]

Thanks, Francis.

[Martin Pool (mbp)]

Could you please merge up trunk and fix the conflicts? Also it may be worth mentioning in the news that there is new information available to the hooks.

[Francis Devereux (frankoid)]

I've merged from trunk and fixed the conflicts.

I separated out the new attribute of StatusHookParams into its own branch and merge proposal at https://code.launchpad.net/~frankoid/bzr/status-hook-params-specific-files/+merge/82930, and that branch has release notes and more tests. I thought it might be good to mark this MP as depending on 82930, but I couldn't find a way to do so (it looked like I could only indicate dependencies when creating a merge proposal, not afterwards). I can merge the other branch into this one if you like.

On 30 Nov 2011, at 00:50, Martin Pool wrote:

> Could you please merge up trunk and fix the conflicts? Also it may be worth mentioning in the news that there is new information available to the hooks.
> --
> https://code.launchpad.net/~frankoid/bzr/status-shelve-message/+merge/82728
> You are the owner of lp:~frankoid/bzr/status-shelve-message.
>

[Martin Pool (mbp)]

You can change the prerequisite branch by clicking 'resubmit' -
there's no need to do that now though,I'll just send both.

[Francis Devereux (frankoid)]

Thanks :-)

Martin Pool <email address hidden> wrote:

You can change the prerequisite branch by clicking 'resubmit' -
there's no need to do that now though,I'll just send both.

--
https://code.launchpad.net/~frankoid/bzr/status-shelve-message/+merge/82728
You are the owner of lp:~frankoid/bzr/status-shelve-message.

=== modified file 'bzrlib/status.py'
--- bzrlib/status.py	2011-06-01 12:53:56 +0000
+++ bzrlib/status.py	2011-11-30 00:53:25 +0000
@@ -157,7 +157,7 @@
         try:
             for hook in hooks['pre_status']:
                 hook(StatusHookParams(old, new, to_file, versioned,
-                    show_ids, short, verbose))
+                    show_ids, short, verbose, specific_files=specific_files))
 
             specific_files, nonexistents \
                 = _filter_nonexistent(specific_files, old, new)
@@ -222,7 +222,7 @@
                 raise errors.PathsDoNotExist(nonexistents)
             for hook in hooks['post_status']:
                 hook(StatusHookParams(old, new, to_file, versioned,
-                    show_ids, short, verbose))
+                    show_ids, short, verbose, specific_files=specific_files))
         finally:
             old.unlock()
             new.unlock()
@@ -416,7 +416,7 @@
     """
 
     def __init__(self, old_tree, new_tree, to_file, versioned, show_ids,
-            short, verbose):
+            short, verbose, specific_files=None):
         """Create a group of post_status hook parameters.
 
         :param old_tree: Start tree (basis tree) for comparison.
@@ -426,6 +426,9 @@
         :param show_ids: Show internal object ids.
         :param short: Use short status indicators.
         :param verbose: Verbose flag.
+        :param specific_files: If set, a list of filenames whose status should be
+            shown.  It is an error to give a filename that is not in the working
+            tree, or in the working inventory or in the basis inventory.
         """
         self.old_tree = old_tree
         self.new_tree = new_tree
@@ -434,14 +437,15 @@
         self.show_ids = show_ids
         self.short = short
         self.verbose = verbose
+        self.specific_files = specific_files
 
     def __eq__(self, other):
         return self.__dict__ == other.__dict__
 
     def __repr__(self):
-        return "<%s(%s, %s, %s, %s, %s, %s, %s)>" % (self.__class__.__name__,
+        return "<%s(%s, %s, %s, %s, %s, %s, %s, %s)>" % (self.__class__.__name__,
             self.old_tree, self.new_tree, self.to_file, self.versioned,
-            self.show_ids, self.short, self.verbose)
+            self.show_ids, self.short, self.verbose, self.specific_files)
 
 
 def _show_shelve_summary(params):
@@ -449,6 +453,10 @@
 
     :param params: StatusHookParams.
     """
+    # Don't show shelves if status of specific files is being shown, only if
+    # no file arguments have been passed
+    if params.specific_files:
+        return
     get_shelf_manager = getattr(params.new_tree, 'get_shelf_manager', None)
     if get_shelf_manager is None:
         return

=== modified file 'bzrlib/tests/blackbox/test_status.py'
--- bzrlib/tests/blackbox/test_status.py	2011-06-03 08:45:36 +0000
+++ bzrlib/tests/blackbox/test_status.py	2011-11-30 00:53:25 +0000
@@ -53,24 +53,25 @@
             status._show_shelve_summary,
             'bzr status')
 
-    def assertStatus(self, expected_lines, working_tree,
+    def assertStatus(self, expected_lines, working_tree, specific_files=None,
         revision=None, short=False, pending=True, verbose=False):
         """Run status in working_tree and look for output.
 
         :param expected_lines: The lines to look for.
         :param working_tree: The tree to run status in.
         """
-        output_string = self.status_string(working_tree, revision, short,
+        output_string = self.status_string(working_tree, specific_files, revision, short,
                 pending, verbose)
         self.assertEqual(expected_lines, output_string.splitlines(True))
 
-    def status_string(self, wt, revision=None, short=False, pending=True,
-        verbose=False):
+    def status_string(self, wt, specific_files=None, revision=None,
+        short=False, pending=True, verbose=False):
         # use a real file rather than StringIO because it doesn't handle
         # Unicode very well.
         tof = codecs.getwriter('utf-8')(TemporaryFile())
-        show_tree_status(wt, to_file=tof, revision=revision, short=short,
-                show_pending=pending, verbose=verbose)
+        show_tree_status(wt, specific_files=specific_files, to_file=tof,
+                revision=revision, short=short, show_pending=pending,
+                verbose=verbose)
         tof.seek(0)
         return tof.read().decode('utf-8')
 
@@ -569,14 +570,22 @@
             ],
             wt)
         self.run_bzr(['shelve', '--all', '-m', 'bar'])
-        self.build_tree(['spam.c'])
+        self.build_tree(['eggs.c', 'spam.c'])
+        wt.add('eggs.c')
         wt.add('spam.c')
         self.assertStatus([
                 'added:\n',
+                '  eggs.c\n',
                 '  spam.c\n',
                 '2 shelves exist. See "bzr shelve --list" for details.\n',
             ],
             wt)
+        self.assertStatus([
+                'added:\n',
+                '  spam.c\n',
+            ],
+            wt,
+            specific_files=['spam.c'])
 
 
 class CheckoutStatus(BranchStatus):

=== modified file 'bzrlib/tests/test_status.py'
--- bzrlib/tests/test_status.py	2011-03-30 11:45:54 +0000
+++ bzrlib/tests/test_status.py	2011-11-30 00:53:25 +0000
@@ -169,7 +169,7 @@
         params = calls[0]
         self.assertIsInstance(params, _mod_status.StatusHookParams)
         attrs = ['old_tree', 'new_tree', 'to_file', 'versioned',
-            'show_ids', 'short', 'verbose']
+            'show_ids', 'short', 'verbose', 'specific_files']
         for a in attrs:
             self.assertTrue(hasattr(params, a),
                 'Attribute "%s" not found in StatusHookParam' % a)
@@ -192,7 +192,7 @@
         params = calls[0]
         self.assertIsInstance(params, _mod_status.StatusHookParams)
         attrs = ['old_tree', 'new_tree', 'to_file', 'versioned',
-            'show_ids', 'short', 'verbose']
+            'show_ids', 'short', 'verbose', 'specific_files']
         for a in attrs:
             self.assertTrue(hasattr(params, a),
                 'Attribute "%s" not found in StatusHookParam' % a)

=== modified file 'doc/en/release-notes/bzr-2.5.txt'
--- doc/en/release-notes/bzr-2.5.txt	2011-11-29 12:57:56 +0000
+++ doc/en/release-notes/bzr-2.5.txt	2011-11-30 00:53:25 +0000
@@ -46,6 +46,9 @@
 * Plugins can now register additional "location aliases".
   (Jelmer Vernooij)
 
+* ``bzr status`` no longer shows shelves if files are specified.
+  (Francis Devereux)
+
 Bug Fixes
 *********