Merge lp:~parthm/bzr/403687-status-shows-shelves into lp:bzr

[Parth Malwankar]

=== Fixes bug #403687 ===
This patch updates the status command to list shelves if any. --short option shows shelves with 'S' in the second column. This patch doesn't do exactly what the bug describes i.e. display files with shelves. Instead it lists shelves with their ids. List of files can probably be looked at in the context of bug #330311.

Shelf listing function cmd_shelf.run_for_list is pulled out into the common location shelf.list_shelves which is used by cmd_shelf and status. list_shelves returns the shelves as a list of strings rather than directly displaying them.

Sample output:

[bzr-grep]% ~/src/bzr.dev/403687-st-shows-shelve/bzr st -S
 M INSTALL
? x
 S 1: <no message>

[bzr-grep]% ~/src/bzr.dev/403687-st-shows-shelve/bzr st
modified:
  INSTALL
unknown:
  x
shelves:
  1: <no message>

[lp:~parthm/bzr/403687-status-shows-shelves]

5370. By Parth Malwankar on 2010-07-28

make imports in bzrlib.shelf lazy to pass test_import_tariff

5371. By Parth Malwankar on 2010-08-01

update status doc to mentioned 'shelved' as a state

5372. By Parth Malwankar on 2010-08-01

merged in trunk and resolved NEWS conflict.

[Vincent Ladeuil (vila)]

I agree with the comments on bug #403687, a single line in the output to mention the number
of shelves would better suit 'bzr status'.

A dedicated command 'bzr shelf-status' or 'bzr shelf --status' or something else
sounds more appropriate for a detailed output.

There are some compatibility issues to consider too here, some programs already parse the output
of status and adding new indicators can't be done lightly.

May be you should bring the subject to the mailing list so it can be discussed more broadly.

review: Needs Fixing

[Parth Malwankar (parthm)]

> I agree with the comments on bug #403687, a single line in the output to
> mention the number
> of shelves would better suit 'bzr status'.
>
> A dedicated command 'bzr shelf-status' or 'bzr shelf --status' or something
> else
> sounds more appropriate for a detailed output.
>
> There are some compatibility issues to consider too here, some programs
> already parse the output
> of status and adding new indicators can't be done lightly.
>
> May be you should bring the subject to the mailing list so it can be discussed
> more broadly.

Yes. This is quite a bit of a change in UI. I will start a discussion in the mailing list.

[Parth Malwankar (parthm)]

I am rejecting this in favor of the hook based and single line suggestion that came up on the mailing list. The proposed post_status hook can be found here https://code.launchpad.net/~parthm/bzr/601749-status-options-show/+merge/33985

Unmerged revisions

5372. By Parth Malwankar on 2010-08-01

merged in trunk and resolved NEWS conflict.

5371. By Parth Malwankar on 2010-08-01

update status doc to mentioned 'shelved' as a state

5370. By Parth Malwankar on 2010-07-28

make imports in bzrlib.shelf lazy to pass test_import_tariff

5369. By Parth Malwankar on 2010-07-28

test case for short status

5368. By Parth Malwankar on 2010-07-28

support for 'st -S' with shelf listing

5367. By Parth Malwankar on 2010-07-28

added blackbox test case

5366. By Parth Malwankar on 2010-07-28

test case cleanup

5365. By Parth Malwankar on 2010-07-28

test case for shelf.list_shelves()

5364. By Parth Malwankar on 2010-07-28

updated NEWS

5363. By Parth Malwankar on 2010-07-28

show 'no changes' message only for shelve command

=== modified file 'NEWS'
--- NEWS	2010-07-26 17:19:33 +0000
+++ NEWS	2010-08-01 07:14:39 +0000
@@ -25,6 +25,8 @@
 New Features
 ************
 
+* ``status`` now lists shelved changes if any. (Parth Malwankar, #403687)
+
 * The ``lp:`` prefix will now use your known username (from
   ``bzr launchpad-login``) to expand ``~`` to your username.  For example:
   ``bzr launchpad-login user && bzr push lp:~/project/branch`` will now

=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py	2010-07-20 10:18:05 +0000
+++ bzrlib/builtins.py	2010-08-01 07:14:39 +0000
@@ -206,8 +206,8 @@
 class cmd_status(Command):
     __doc__ = """Display status summary.
 
-    This reports on versioned and unknown files, reporting them
-    grouped by state.  Possible states are:
+    This reports on files and changes, reporting them grouped by state.
+    Possible states are:
 
     added
         Versioned in the working copy but not in the previous revision.
@@ -230,6 +230,9 @@
     unknown
         Not versioned and not matching an ignore pattern.
 
+    shelved
+        Shelved changes.
+
     Additionally for directories, symlinks and files with an executable
     bit, Bazaar indicates their type using a trailing character: '/', '@'
     or '*' respectively.
@@ -5844,7 +5847,17 @@
     def run(self, revision=None, all=False, file_list=None, message=None,
             writer=None, list=False, destroy=False, directory=u'.'):
         if list:
-            return self.run_for_list()
+            from bzrlib.shelf import list_shelves
+            tree = WorkingTree.open_containing('.')[0]
+            self.add_cleanup(tree.lock_read().unlock)
+            shelves = list_shelves(tree.get_shelf_manager())
+            for s in shelves:
+                self.outf.write(s)
+            if shelves:
+                return 1
+            else:
+                note('No shelved changes.')
+                return 0
         from bzrlib.shelf_ui import Shelver
         if writer is None:
             writer = bzrlib.option.diff_writer_registry.get()
@@ -5858,21 +5871,6 @@
         except errors.UserAbort:
             return 0
 
-    def run_for_list(self):
-        tree = WorkingTree.open_containing('.')[0]
-        self.add_cleanup(tree.lock_read().unlock)
-        manager = tree.get_shelf_manager()
-        shelves = manager.active_shelves()
-        if len(shelves) == 0:
-            note('No shelved changes.')
-            return 0
-        for shelf_id in reversed(shelves):
-            message = manager.get_metadata(shelf_id).get('message')
-            if message is None:
-                message = '<no message>'
-            self.outf.write('%3d: %s\n' % (shelf_id, message))
-        return 1
-
 
 class cmd_unshelve(Command):
     __doc__ = """Restore shelved changes.

=== modified file 'bzrlib/help_topics/__init__.py'
--- bzrlib/help_topics/__init__.py	2010-06-02 04:50:35 +0000
+++ bzrlib/help_topics/__init__.py	2010-08-01 07:14:39 +0000
@@ -581,6 +581,7 @@
   D File deleted
   K File kind changed
   M File modified
+  S Shelf
 
 Column 3 - execute::
 

=== modified file 'bzrlib/shelf.py'
--- bzrlib/shelf.py	2010-04-30 11:03:59 +0000
+++ bzrlib/shelf.py	2010-08-01 07:14:39 +0000
@@ -15,6 +15,8 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 
 
+from bzrlib.lazy_import import lazy_import
+lazy_import(globals(), """
 import errno
 import re
 
@@ -26,6 +28,7 @@
     pack,
     transform,
 )
+""")
 
 
 class ShelfCreator(object):
@@ -429,3 +432,21 @@
             return active[-1]
         else:
             return None
+
+
+def list_shelves(manager):
+    """Return the current list of shelves as strings.
+
+    :param manager: Shelf manager for the tree.
+    """
+    shelves = manager.active_shelves()
+    output = []
+    if len(shelves) == 0:
+        return output
+    for shelf_id in reversed(shelves):
+        message = manager.get_metadata(shelf_id).get('message')
+        if message is None:
+            message = '<no message>'
+        output.append('%3d: %s\n' % (shelf_id, message))
+    return output
+

=== modified file 'bzrlib/status.py'
--- bzrlib/status.py	2010-04-30 11:35:43 +0000
+++ bzrlib/status.py	2010-08-01 07:14:39 +0000
@@ -24,6 +24,7 @@
     revision as _mod_revision,
     )
 import bzrlib.errors as errors
+from bzrlib.shelf import list_shelves
 from bzrlib.trace import mutter, warning
 
 # TODO: when showing single-line logs, truncate to the width of the terminal
@@ -210,6 +211,15 @@
                 show_pending_merges(new, to_file, short, verbose=verbose)
             if nonexistents:
                 raise errors.PathsDoNotExist(nonexistents)
+            shelves = list_shelves(wt.get_shelf_manager())
+            if shelves:
+                prefix = ''
+                if not short:
+                    to_file.write("shelves:\n")
+                else:
+                    prefix = ' S'
+                for s in shelves:
+                    to_file.write(prefix + s)
         finally:
             old.unlock()
             new.unlock()

=== modified file 'bzrlib/tests/blackbox/test_status.py'
--- bzrlib/tests/blackbox/test_status.py	2010-06-24 06:57:31 +0000
+++ bzrlib/tests/blackbox/test_status.py	2010-08-01 07:14:39 +0000
@@ -33,6 +33,7 @@
     conflicts,
     errors,
     osutils,
+    shelf,
     )
 import bzrlib.branch
 from bzrlib.osutils import pathjoin
@@ -520,6 +521,30 @@
         out, err = self.run_bzr('status branch1 -rbranch:branch2')
         self.assertEqual('', out)
 
+    def test_shelve_status(self):
+        """Ensure that shelves are lists.
+        """
+        tree = self.make_branch_and_tree('.')
+        self.build_tree(['hello.c', 'bye.c'])
+        creator = shelf.ShelfCreator(tree, tree.basis_tree())
+        self.addCleanup(creator.finalize)
+        shelf_manager = tree.get_shelf_manager()
+        shelf_id = shelf_manager.shelve_changes(creator)
+        self.assertStatus([
+                'unknown:\n',
+                '  bye.c\n',
+                '  hello.c\n',
+                'shelves:\n',
+                '  1: <no message>\n',
+            ],
+            tree)
+        self.assertStatus([
+                '?   bye.c\n',
+                '?   hello.c\n',
+                ' S  1: <no message>\n',
+            ],
+            tree, short=True)
+
 
 class CheckoutStatus(BranchStatus):
 

=== modified file 'bzrlib/tests/test_shelf.py'
--- bzrlib/tests/test_shelf.py	2010-01-13 23:06:42 +0000
+++ bzrlib/tests/test_shelf.py	2010-08-01 07:14:39 +0000
@@ -729,6 +729,23 @@
         unshelver.make_merger().do_merge()
         self.assertFileEqual('bar', 'tree/foo')
 
+    def test_list_shelves(self):
+        """Ensure shelf.list_shelves() works correctly.
+        """
+        tree = self.make_branch_and_tree('tree')
+        tree.commit('no-change commit')
+        tree.lock_write()
+        self.addCleanup(tree.unlock)
+        self.build_tree_contents([('tree/foo', 'bar')])
+        tree.add('foo', 'foo-id')
+        creator = shelf.ShelfCreator(tree, tree.basis_tree())
+        self.addCleanup(creator.finalize)
+        shelf_manager = tree.get_shelf_manager()
+        self.assertEqual([], shelf.list_shelves(shelf_manager))
+        shelf_id = shelf_manager.shelve_changes(creator)
+        self.assertEqual(['  1: <no message>\n'],
+            shelf.list_shelves(shelf_manager))
+
     def test_get_metadata(self):
         tree = self.make_branch_and_tree('.')
         tree.lock_tree_write()