[PATCH 1 of 8] extdoc: separate module for handling extractions of descriptions

Cedric Duval cedricduval at free.fr
Tue Jul 7 01:55:35 CDT 2009


Martin Geisler wrote:
> Cédric Duval <cedricduval at free.fr> writes:
> > -        doc = (gettext(ext.__doc__) or _('(no help text available)'))
> > +        doc = ext.__doc__ or '(no help text available)'
> 
> It's unfortunate to have a raw string without the _(...) function. It
> will only be translated because the same string occurs in commands.py.
> So I think there should at least be a comment.

Actually the right thing to do here is use N_().

> > -def disabled():
> > +def disabled(raw=False):
> 
> I was not able to find anything that actually calls disabled and
> enabled with raw=True. So maybe these arguments are not needed?

extdoc alone is used with raw=True, in patches 6 and 8. It is only
here in disabled because I think Dan's patch for displaying the help
of non-activated extensions would have needed it, and in enabled by
symmetry. Yes, that could be removed until someone actually uses it.

> > +        if not raw:
> > +            doc = extdoc.synopsis(doc)
> 
> Could we not let it be up to the caller of extractdoc to shorten the doc
> to the first line?
> 
> Then extractdoc could always return a localized version of the full
> documentation. When building the hg.1 manpage the caller should prepare
> the environment to be in the C locale so that we get the same result as
> if we passed the 'raw' flag you have.

Possibly yes, though is a setlocale dance (in both gendoc.py and
setup.py) really simpler than passing a flag?

> > +    os.chdir('..')
> > +    for name, desc in sorted(extdoc.extract('hgext', raw=True).iteritems()):
> 
> Would it work to pass '../hgext' as the path instead of doing the
> os.chdir back and forth?

Ah, the horrible chdir dance! '../hgext' used not to be enough, but
later I have changed extdoc.files to use abspath and now indeed it
would work. :)

> While looking at the patches I began wondering how much they actually
> change. It turns out to be less than I expected:
[...]
> I think I would prefer if some of the refactor-first-then-move patches
> were folded together into a move-and-refactor patch. But maybe that's
> just me?

Yes, I wanted to keep small functional chunks for easy reviewing, but
in hindsight I believe it might perhaps have been better folded.

Not so much new code in fact, merely this quite verbose refactoring.
But once things have been moved into extdoc, everything falls into
place naturally, and the same code is re-used by hg, gendoc and setup.

> >  - patch 8: new generated section in the hg.1 presenting each extension
> 
> After asking a ton of questions in my other mails, let me just finish by
> saying that it actually works... :-) It's very cool to have a
> description of all the extensions in the hg.1 man page.

Glad you like it. :) Though the resulting (non-)wrapping in the man page
is less than optimal, when read in a 80 characters wide terminal...

> Matt Mackall <mpm at selenic.com> writes:
> > Still not really excited about this whole approach. I'd actually
> > rather import modules (and tweak all the built-ins to be
> > random-import-friendly) than make our build uglier.
> 
> I must admit that I also think this is simpler -- I just somehow
> thought that this option was rejected earlier in the thread where Dan
> came up with the pydoc-solution...

Yes, it had been my understanding that the import approach was not deemed
acceptable, that's why we went out of our way with the parsing approach,
which is necessarily a bit more complex.

Not overly complex and the relevants parts are quite localized, but what
I'm the less pleased about is the need for the pre-processed file for the
frozen library issue (which incidentally makes for an ugly grepping of
the sources).

So what should I do then? Improve on those, or make a U-turn and go
back to importing?

> As I see it, the only problem with importing modules is to figure out
> the list of importable modules. That we can do a build-time and amend it
> at run-time with os.listdir if we're dealing with normal .py files.

That might be a compromise. Then with third-party extensions found
at runtime, parsing or importing?

Thanks a lot for the review.
-- 
Cédric


More information about the Mercurial-devel mailing list