RFE: Add 'doc' attribute to IAccountRegistrationInspector implementation

[Steffen Hoffmann]

Spamfilter has been the first external code base that implemented the modular user registration process of ​th:wiki:AccountManagerPlugin.

There is a greatly improved configuration admin panel on the way for acct_mgr-0.5, that makes use of a new 'doc' attribute, even rendering it according to WikiFormatting rules. When it was brought to my attention, that the configuration was broken by spamfiler, I implemented a fallback, but it would be great to adjust the implementation to the current API and implement that attribute appropriately for better integration. And it doesn't hurt to disclose some more information to potential users too. Patch​ included.

[Steffen Hoffmann]

add requested component documentation including WikiFormatting to fit into upcoming ​th:wiki:AccountManagerPlugin configuration admin panel

[Dirk Stöcker]

In r11702.

[Christian Boos]

The N_(self.__class__.__doc__) is wrong…

[Steffen Hoffmann]

Replying to cboos:

The N_(self.__class__.__doc__) is wrong…

Well, that would be my fault. I'm simply not aware of any other method to reuse classes Python doc-string for the web-UI, not even in Trac 1.0, that is capable of extracting option descriptions now.

So what did you have in mind here? Shall we duplicate these strings for i18n, or is it allowed to wrap the doc-string directly instead so that returning just self.__class__.__doc__ would be enough? I'm eager to learn, because I ​did it similar for all components implementing that interface.

[Christian Boos]

N_ is the no-op i18n marker for extraction. Here you have no string to mark, hence it's even more a no-op ;-)

What you wanted instead was gettext(self.__class__.__doc__), assuming that docstring was itself marked for extraction. Which we can't do, so we have to use tricks like what we do for TracIni options or WikiMacros, see e.g. cleandoc_ usage​.

[Steffen Hoffmann]

rework doc-string following the approach for WikiMacros in [10617]

[Steffen Hoffmann]

Thanks cboos for another valuable lesson on i18n stuff. I should have looked into it more closely before.

As mentioned in ​th:changeset:12721 using Trac's extract_python works, but looks a bit awkward for not being able to gracefully skip the javascript and 'tracini' compiler runs. Is it worth to make it slightly more (re-) usable with Trac plugins in mind, or did I miss something again?

I'll wait for your suggestion before providing a patch to actually extract the message from new cleandoc_ part.

[Jun Omae]

Yes. The trac.dist:extract_python is already re-usable with plugins. However, get_l10n_trac_cmdclass is no need for it. The method is for Trac core.

See ​https://github.com/jun66j5/accountmanagerplugin/compare/trunk...ticket8930.

[Steffen Hoffmann]

Replying to jomae:

Yes. The trac.dist:extract_python is already re-usable with plugins. However, get_l10n_trac_cmdclass is no need for it. The method is for Trac core.

See ​https://github.com/jun66j5/accountmanagerplugin/compare/trunk...ticket8930.

Thanks. Didn't even try to just continue using get_l10n_cmdclass. Much better now, thanks. Still I'm note so keen to call get_l10n_cmdclass() unconditionally, but this shouldn't be a problem.

Well, current SpamFilterPlugin shouldn't have an issue using trac.dist:extract_python right-away, so I'll propose that now.

[Steffen Hoffmann]

Oh, I see, this is already done. Nothing more than the second patch should be required to get component's doc translated in AccountManager's new configuration admin panel.

[Dirk Stöcker]

Sorry, I'm lost. What needs to be applied now?

[Steffen Hoffmann]

Replying to dstoecker:

Sorry, I'm lost. What needs to be applied now?

Ah, sorry for the probably ambiguous comment.

Jun helped me to get it right, and after I understood the 'doc' method was no longer required. The second patch is a cumulative one. Please apply it over the already committed first one, and it should work just fine. Of course, for translated messages extracting the doc-string marked by 'cleandoc_' extraction marker to the tracspamfilter catalog will be required too. Shall I provide this as part of the patch too?

[Dirk Stöcker]

In r11714, but text is not transalted yet. There must still be an issue. I use accountmanager trunk 12739.

[Steffen Hoffmann]

Sure, as pointed out in my last comment, the message extraction and translation must happen within SpamFilterPlugin. AccountManager doesn't know about the extra component's messages, so default English text will be shown.

[Dirk Stöcker]

You misunderstood, see checkin. Text is extracted and contained in the translations file (for German), domain is set correctly, but still translation is not shown.

[Christian Boos]

Well, probably because the ​TH:AccountManagerPlugin doesn't use the _domain information yet… Seeing _domain = 'messages' in the patch looks suspicious so I did a checkout of ​accountmanagerplugin/trunk and saw no use of _domain.

To complement comment:4, see also how the _domain and _description values are used by the dgettext call in MacroListMacro​ (via WikiMacroBase.get_macro_description()).

[Dirk Stöcker]

Hmm, when this is subject to change anyway, shouldn't it be "_doc_domain" instead of "_domain"?

Ah sorry, _domain is already used elsewhere.

[Steffen Hoffmann]

True, I had to use dgettext and the _domain attribute, but this was not the only thing that needed a correction.

It works now starting with ​th:r12751 of the AccountManager. Sorry for not testing this before. Finally got the spamfilter plugin installed now in my test environment. And thanks for you patience with me trying to get things straight and working at my side in the first place.

[Dirk Stöcker]

These changes now have one trouble. In the plugin handling of Trac the description text vanished.

Any suggestions for that issue?

[Steffen Hoffmann]

I've seen this side-effect too. Duplicating at least the first line to restore the initial doc-string would be hackish, but I can't see another short-term fix.

Ultimately I'd suggest to improve the logic behind plugin admin panel to consider the _description attribute as alternative to class doc-strings, or even the other way round, aiming at translating that page's content too?

[Dirk Stöcker]

Can't the text be assigned to __doc__ and domain be "doc_domain"? That would be identical to the method of the Trac options. ALso __doc__ would then be english string and could be translated when code knows about i18n.

Does not break compatibility as much as current method.

[Steffen Hoffmann]

I understand, that you dislike attaching the component description to something else than the doc-string, making it vanish from Trac's plugin admin panel. OTOH it gives us more control, i.e. allowing to display a shorter description in the admin panel, the more verbose in AccountManager's configuration admin panel alone.

All relevant strings need to get marked for extraction. As far as I understand the solution for Trac WikiMacros, it has been used as a workaround, because the extraction happens on the plain text file, not instantiating any Python object. Consequently __doc__ attribute is not available for extraction. Remember that referring to doc-strings was my initial attempt, but as a matter of fact I was unable to extract doc-strings, confirming that assumption and explaining Christians statement "Here you have no string to mark" in comment:4.

However I do not understand your complaint about how to give the message domain. 'Section.doc_domain' attributes from configuration wouldn't help anything here, because the component is not tied to a dedicated configuration section like Trac and Trac plugin options are. And self._domain attribute doesn't do any harm at all.

Furthermore no complaint has been raised against my suggestion, so the enhancement to plugin admin panel code looks like the best way to re-get description there, but the more I think about it the more I doubt, that Christian an others even desire the full macro description to appear there.

[Dirk Stöcker]

However I do not understand your complaint about how to give the message domain.

It is not a complaint, more a unification attempt.

[Dirk Stöcker]

In r11751 - readded a second comment.

For the other suggestion a new ticket against core may be useful (or not?).