Antonio Terceiro wrote:
Hi,
Stefano escreveu isso aí:
I'm attaching you a patch for freedesktop package to show also dirs
and icons contained in ~/Desktop. It parses the system MIME types to
find the right icons. Please comment it.
It looks nice, thanks for working on that. I have some comments, though:
0) would you please rebase your work off the current code. It seems you
reverted several recent changes to the code organization. In special, stick to
the current interface of the functions (i.e. using actual, explicit arguments
instead of single-table-argument functions)
This is a common tecnique to avoid passing a lot of default parameters
to functions (js docet :)). Also, this is useful for preserving a fixed
but easily extendible interface, as we're still in a very experimental
state. I personally suggest to preserve it.
1) please make sure there are no trailing whitespace. git diff --check helps
with that. (plus a plain git diff will highlight trailing whitespace if you
have color enabled (`git config --global color.ui auto`).
Sorry for that, I was in a hurry! :P
2) I'm not sure it's usefull to add explictly both application and file icons.
Why don't we have a single add_destkop_icons that does both things, but without
exposing it to the user?
We could do it, if you think this is a level of custumization not needed
by the average awesome user. Two separate functions is a more modular
approach to the problem. We just have to keep those functions local, and
expose a merged add_desktop_icons() function that calls both of them.
3) I also don't like splitting the icon lookup like that. Finding an icon must
be a unique operation. Mind you, some applications actually use mimetype icons
as icons for their launchers, so I don't differentiating the icon lookup like
that will help us.
Maybe I'm wrong, but the process of finding a .desktop icon is very
different from finding a file icon, just look at the code. The same
applies to directory icons. Again, I propose 3 different local functions
to modularize the code. We could then have a callable "merged" function.
Obviously there's still some refactoring to be done there :)
The only problem is it doesn't get refreshed on FS change.. any way I
could solve it?
Maybe registering a function with the timer, just like the one which updates
the clock etc.
awful.hooks.timer.register(1, function ()
-- check for updates in ~/Desktop
end)
But not every 1 second, I guess. ;-)
I thought about that, but that's not a great solution, as changes
wouldn't be instantaneously catched. Other hints anyone else?
Thanks,
Stefano Verna
http://www.downthemall.net
--
To unsubscribe, send mail to [email protected].
