On Sun, Jun 21, 2009 at 2:40 PM, Mads Kiilerich<[email protected]> wrote:
> Here are some comments from my new attempts at creating an tortoisehg Fedora
> rpm.
>
> To make a long story short I will just be impolite and buzz in and make
> complaints and comments without having done proper research. Please shoot
> back and let me know what you think.

> TortoiseHg uses hggtk and thgutil from the python package namespace without
> any good reasons. Please place all the the libs below the tortoisehg
> namespace - which also happens to be the name used by the egg.

This may not be a bad idea, if a bit pedantic.  We can do it after the
0.8 release.

> It is a bit confusing that the rpm package "tortoisehg" mainly provides a
> binary called "hgtk". Perhaps it would be an idea to introduce the name
> "thg" and slowly move to that and deprecate "hgtk"?

hgtk is the GTK implementation of the TortoiseHg dialogs.  thgutil has
utilities shared by hgtk, the nautilus extension, and the thgtaskbar
application on windows.  There may be other implementations in the
future (Qt is being considered).

> A unix commandline tool like "hgtk" shouldn't fork. We need a way to disable
> that by default, prefably without patching the code. A setting in
> "__paths__.py" would be fine.

I beg to differ, many apps that are primarily GUI fork a background
process.  As far a mechanisms to disable it, there's currently
--nofork and HGTK_NOFORK.  Adding further options would be ok.

> Is names like "__paths__.py" really a good idea? That kind of names are
> reserved for Python, for example there is thgutil.__path__ which is
> confusingly close to thgutil.__paths__. I would suggest using "custom.py" or
> "config.py" instead.

The underscores were mainly to indicate the file is machine generated,
like __version__.py.  The exact name of it is not that important.

> When building an rpm then the steps source preparation, building and
> installing should be separated, so I would like to create
> "thgutil/__paths__.py" before running setup.py. Could setup.py be taught to
> only create/overwrite the file when told so?

Fixed.

> setup.py installs locale file with an extra locale in the path, such as
> /usr/share/locale/locale/da/LC_MESSAGES/tortoisehg.mo

Fixed

> With that fixed localication works, but slightly annoying messages
> indicatest that there might be a real problem:
> [...@localhost tortoisehg-crew]$ LANG=da hgtk --nofork log
> (process:8174): Gtk-WARNING **: Locale not supported by C library.
>    Using the fallback 'C' locale.
> Is that seen on other platforms too?
>
> And finally a windowsism-crash from running vdiff:
> ** Please report this bug to [email protected] or
> http://bitbucket.org/tortoisehg/stable/issues
> ** Mercurial version (7951f385fcb7).  TortoiseHg version (96f7aaae3e59)
> ** Command: --nofork vdiff
> ** CWD: /home/mk/tortoisehg-crew
> ** Extensions loaded: patchbomb, graphlog, mq, extdiff
> Recoverable runtime error (stderr):
> Traceback (most recent call last):
>  File "/usr/lib/python2.6/site-packages/hggtk/visdiff.py", line 215, in
> rowactivated
>    self.launch(*model[paths[0]])
>  File "/usr/lib/python2.6/site-packages/hggtk/visdiff.py", line 241, in
> launch
>    except (WindowsError, EnvironmentError), e:
> NameError: global name 'WindowsError' is not defined

Fixed.

> It is slightly annoying/confusing that no indication of problems is shown
> before the dialog is closed. If shit happens then I would prefer to be
> informed ASAP.

Again, this is a Windowsism.  GTK likes to spew useless messages to
stderr and on Windows this causes an error window to appear and a log
file to be generated at exit.  To defend ourselves from this hgtk
pipes stderr to a cStringIO instance and parses the contents at exit
and pops up that bugreport dialog if it finds any "real" errors.

This behavior can be disabled by a THGDEBUG environment variable.
Perhaps it could be made platform specific as well.

> And some minor comments from my quick test as thg newbie:
>
> The About/License window looks bad when the GPL text already is linewrapped
> and it is shown in a window smaller than the wrapping. Could the window be
> made twice as wide?

Could be.  But the width needs to be platform specific.  On Windows
the text fits nicely the way it is (Windows likes to scale their fonts
differently than everyone else).

> "hgtk add" apparently adds all unknown files? The help text is not very
> helpful. What can it do that "hg addremove" can't?

Absolutely nothing.  hgtk add is only there for the windows context
menu, which can only run hgtk commands.  It's a convenient way to add
files selected in the file explorer.  If you want to selectively add
files, it's best to use the status or commit tools.

> Esc closes some dialogs, e.g. userconfig. That is nice. But my fingers gets
> confused when they can't close other windows which looks like temporary
> dialogs. I would expect Esc to close most dialogs - perhaps except for the
> commit dialog with a pending message.

We have ctrl-w and ctrl-q shortcuts for closing windows and
applications accordingly.  It's Windows/Mac centric.  I have mixed
thoughts on Esc closing apps.  For transient dialogs, yes, but I find
my VIM fingers hitting escape at inopportune times and am glad they do
not close my open apps.

--
Steve Borho

------------------------------------------------------------------------------
Are you an open source citizen? Join us for the Open Source Bridge conference!
Portland, OR, June 17-19. Two days of sessions, one day of unconference: $250.
Need another reason to go? 24-hour hacker lounge. Register today!
http://ad.doubleclick.net/clk;215844324;13503038;v?http://opensourcebridge.org
_______________________________________________
Tortoisehg-develop mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/tortoisehg-develop

Reply via email to