ls: show directories before files

[GoogleCodeExporter]

ls should provide the option to show directories before files as nautilus
and other file managers do. the attached patch adds a sort to the builtin
ls. one could also do this optionally and add an option to preferences. in
the future, search by size, type, ... may be added.

Original issue reported on code.google.com by KaiSchro...@gmail.com on 24 Jan 2008 at 4:44

Attachments:

• directories_first.patch

[GoogleCodeExporter]

Hmm.  I'm not sure I want to make it the default.  Currently the "ls" builtin is
trying to stay closer in concept to how "/bin/ls" operates than how Nautilus 
does.

Let's look at what "/bin/ls" does.  On my system, the GNU coreutils "ls" has an
option "--group-directories-first".  That's kind of a long name, but we could 
just
take over one of the less-commonly used options like "-g"?

Original comment by cgwalt...@gmail.com on 24 Jan 2008 at 5:20

• Changed state: Accepted
• Added labels: Type-Patch
• Removed labels: Type-Defect

[GoogleCodeExporter]

The other thing we should really do is make clicking on the treeview columns do 
sorting.

Original comment by cgwalt...@gmail.com on 24 Jan 2008 at 6:18

[GoogleCodeExporter]

Yes, I have thought about that too - I will have a look at it over the weekend.

Original comment by KaiSchro...@gmail.com on 24 Jan 2008 at 7:06

[GoogleCodeExporter]

I have a new patch that adds sorting. I should probably mention that I am new 
to both
Python and GTK :) I have not yet verified if TreeModelSort (which is now used 
as the
standard tree view model) is a good choice for other renderers than File.  The
sorting function is quite long at the moment - one might want to think about
splitting that up into individual functions if profiling shows that its worth 
doing that

Original comment by KaiSchro...@gmail.com on 27 Jan 2008 at 4:41

Attachments:

• sort.patch

[GoogleCodeExporter]

Wow!  This is a great patch, period.  But if you just learned Python/GTK+ that's
quite impressive!  Some comments:

*) On the preference UI, one thing I am hoping is that we can move to a more 
directly
interactive and stateful system for result renderers.  See issue 113.  So the 
idea
here would be that if you once clicked to sort on the mime type for File 
objects, we
would remember that.  We could implement the saving part without landing the
add/remove column bits actually now. We would probably want a right-click menu 
at
least to reset to the default state.  What do you think? 

Though I guess there's no clean way to express directories-before-other in the
columns as they are.  Though maybe we could have the icon column sort that way 
rather
than being a mime type duplicate?

*) For the string sorting:

+    def __get_column_value(self, obj, column):
+        compare_obj = {
+            0 : lambda x: x.path.lower(),

I wonder if there's a difference between cmp() on unicode strings and 
locale.strcoll?
From looking at the Python sources, it seems like this ends up calling
Objects/stringobject.c:string_richcompare() which just uses memcmp(), but it's 
unclear.

Ah actually, looking at the locale module, there is a specific function to 
create a
collation key from a unicode string - strxfrm().  So we should be using that 
instead
of string.lower I think.

* Also in the sorting function:
+        prefs = Preferences.getInstance()
+        if (prefs.get_pref('hotwire.ui.render.File.general.foldersbeforefiles',
default=True)):

This is expensive.  The prefs.get_pref is going to do a SQLite query each time 
it's
called.  We should probably cache the current value as 
self.__folders_before_files or
something.

* In the view constructor:

+            'path': ('Path', self._render_objtext, cell_renderer, 1), 

Missing gettext call _('Path') here.  Hopefully at some point we'll have 
translations..

Original comment by cgwalt...@gmail.com on 27 Jan 2008 at 5:52

[GoogleCodeExporter]

Your patch shouldn't block on the persistence thing by the way; I think to land 
that
cleanly we will probably need some other core changes, like adding UUIDs to 
renderers
so we can have a way to cleanly store metadata about them on disk without 
relying on
e.g. the module path which could change.

Original comment by cgwalt...@gmail.com on 27 Jan 2008 at 5:56

[GoogleCodeExporter]

> Wow!  This is a great patch, period.  But if you just learned Python/GTK+ 
that's
> quite impressive!  Some comments:

Thanks - being a C++ programmer using the boost libraries the concepts weren't 
knew
to me, though :)

>*) On the preference UI, one thing I am hoping is that we can move to a more 
>directly
>interactive and stateful system for result renderers.  See issue 113.  So the 
idea
>here would be that if you once clicked to sort on the mime type for File 
objects, we
>would remember that.

I agree, that this choice should be remembered. 

> We could implement the saving part without landing the
>add/remove column bits actually now. We would probably want a right-click menu 
at
>least to reset to the default state.  What do you think? 

Yes, a right click menu to configure renderers should be implemented...

>Though I guess there's no clean way to express directories-before-other in the
>columns as they are.  Though maybe we could have the icon column sort that way
>rather than being a mime type duplicate?

I do think, however, that file management is such an important task that it 
deserves
an additional mention in the preference settings (I am going to write a bit more
about that to the google group in a few days). Actually, I would prefer to 
remove the
icon column completely and somehow add the icon to the name / path renderer. To 
abuse
it for the "folders before files function" sounds a bit too hacky / unintuitive 
:)

> *) For the string sorting:
[...]
> Ah actually, looking at the locale module, there is a 
> specific function to create a
> collation key from a unicode string - strxfrm().  So we should be using that
> instead of string.lower I think.

Hm - .lower() is probably not a good choice - but strxfrm also doesn't seem to 
be one
( http://bugs.python.org/issue1618 )

> * In the view constructor:
>
> +            'path': ('Path', self._render_objtext, cell_renderer, 1), 

> Missing gettext call _('Path') here.  Hopefully at some point we'll have
> translations..

thanks, done

Original comment by KaiSchro...@gmail.com on 28 Jan 2008 at 2:03

Attachments:

• sort2.patch

[GoogleCodeExporter]

I also have an extended version of that patch to add / remove columns (very 
similar
to issue 113, but I hope it's OK when I post it here instead of splitting the 
patch
up). Depending on the tree view, it might be more efficient to recreate the 
whole
view without these columns instead of hiding them. That probably depends on the
amount of rows and the amount of columns - but this is a start. 

Original comment by KaiSchro...@gmail.com on 28 Jan 2008 at 2:25

Attachments:

• sort3.patch

[GoogleCodeExporter]

This patch looks great.  Can I add you as a committer?

One small style detail:

+        if fobj1 > fobj2:
+            return 1
+        elif fobj1 < fobj2:
+            return -1
+        else: # ==
+            return 0

Can be replaced by: return cmp(fobj1, fobj2)

Original comment by cgwalt...@gmail.com on 28 Jan 2008 at 3:22

[GoogleCodeExporter]

you mean svn commit access? well, sure. As long as I am still learning
python/gtk+/hotwire I am thankful when someone has a look at my code - so I 
would
still prefer to send patches to the issue tracker and only commit them 
afterwards
(probably you meant that anyway). If you want to send me a password, my email 
address
starts with schroed@ (the rest is readable in that google groups username - I 
wonder
why google doesn't show my nickname here - perhaps because I didn't choose a 
unique one?)

Can you please point me to a link that explains the naming convention for the
underscore prefixes of methods?

Original comment by KaiSchro...@gmail.com on 28 Jan 2008 at 4:47

[GoogleCodeExporter]

Yeah, all I needed was the full email; the way Google Code works is you get a 
private
password to commit to projects.  You should have commit access now.

For committing patches, yeah the idea is that I'd like to use the issue tracker 
for
peer review.  I wrote up some guidelines this morning:
http://code.google.com/p/hotwire-shell/wiki/Contributing

For the underscore conventions, see:
http://code.google.com/p/hotwire-shell/wiki/CodingStyle

The underscore conventions in Python I'm not totally happy with, but there's no
perfect language..

Original comment by cgwalt...@gmail.com on 28 Jan 2008 at 5:17

[GoogleCodeExporter]

I should have actually tried this patch rather than just running it =)  There 
was a
bug where we were using the same cell renderer for every column; they all need 
to be
separate objects.  

Also I decided not to use the Link renderer anymore, it isn't really worth being
different.

Committed r911
    M   hotwire_ui/renderers/file.py
r911 = cb2158d13c7704b38b552cdbf207b6f1ca8fcba3 (git-svn)

Original comment by cgwalt...@gmail.com on 28 Jan 2008 at 7:34

• Changed state: Fixed