Modify

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#3861 closed enhancement (fixed)

MultiTOC Macro? The original TOC macro ignores "Negotiating" suffixes

Reported by: izzy Owned by: gotoh
Priority: normal Component: TracWikiNegotiatorPlugin
Severity: normal Keywords:
Cc: Trac Release: 0.11

Description

Since coderanger refuses to adapt the TOC macro accordingly (and I guess he's right with that), I move my issue from #3860 here: Could you please add an equivalent to the TOC macro which is handling the negotiation stuff as well - while supporting the same syntax (and features) as the TOC macro does? Maybe it is possible to re-use code from there.

Attachments (4)

helptext.patch (1.0 KB) - added by izzy 6 years ago.
Patch on macros.py for help text correction and disabling version check
macros.py.patch (2.0 KB) - added by izzy 6 years ago.
Cumulative patch (includes helptext.patch) to finally make the [[NTOC]] macro work
macros.py.replace_toc.patch (2.2 KB) - added by izzy 6 years ago.
Cumulative patch (including the previous two), making [[TOC]] an alias to [[NTOC]] (as described in comment:1)
macros.py.default_lang.patch (1.3 KB) - added by izzy 6 years ago.
Using default_lang instead of 'other' in MultiLangTitleIndex? (Patch against r4424)

Download all attachments as: .zip

Change History (22)

comment:1 Changed 6 years ago by gotoh

  • Resolution set to fixed
  • Status changed from new to closed

I've added a new macro NTOC macro delived from TOC macro.
'N' means negotiated :-).
I hope it presents almost of the behaviour you expected in #3860
except wildcard support.

BTW, as experimental tips, if you want replace TOC macro to NTOC
without editing pages, edit source and add following lines to redefine TOC macro:

class TOCMacro(NTOCMacro):
    pass

comment:2 Changed 6 years ago by anonymous

Thank you for the fast reaction! But could you please explain where to insert that code? I just added it at the end of macros.py - the egg was build fine, but the server then always gives an "error 500" - where the last line from debug log is "Loading TracWikiNegotiator" - so it must obviously be the wrong place.

Moreover, before recommending the patch it maybe should be noted what you wrote in macros.py concerning wildcards:

TOC macro accepts wildcard like Trac* to list multiple pages but NTOC macro cannot hook it. In such case, all the variants will match and be used. It'd be better not to use wildcard with NTOC macro.

But OTOH: What difference would it make in this case whether you have the wildcards in the TOC macro, or in the NTOC macro? Result would be pretty the same, I guess? And what about e.g. Trac*.en?

Due to above mentioned problems, I installed without the patch. But using [[NTOC]] just gives an: Error: Failed to load processor NTOC? I checked the plugin section on the admin page (expecting I have to enable the macro first), but in TracWikiNegotiator 1.2 I only see MultiLangTitleIndex and WikiNegotiator. Is there something missing? Or could it be that this explains the 500 error: Since the patch does not have a "try", it simply fails since the NTOC macro could not be found?

Two additional questions: Does the TOCMacro have to be installed and enabled (it is here), installed and disabled, or must it not be installed (I guess one of the first two options fit, since it looks like you import from there)? And: Is there anything else the NTOCMacro depends on which needs to be installed? Trac Version is from SVN stable about a week ago.

Any help appreciated ;)

comment:3 Changed 6 years ago by izzy

I must admit, I'm not a Python programmer - but I played a bit with the code. I added some debug messages and found out, the entire block was skipped - until I commented out the line

assert [int(x) for x in version.split('.')] >= [0, 11] # need 0.11

Guess it got confused by the SVN version which could have been appended (here: 0.11stable-r7572) - trying int('11stable-r7572') results in ValueError: invalid literal for int() with base 10: '11stable-r7572'. Second, I added

from macros import NTOCMacro

to __init.py. Now NTOCMacro showed up in the plugin list of the admin page, as component of TracWikiNegotiator, so I could enable it.

So far, so good. Next I tried to use it, and just added a simple [[NTOC]] to some Sandbox article. The error about the macro not to be found was no longer displayed - but instead I got:

Error: Macro NTOC(None) failed

   get_page_text() takes exactly 3 arguments (2 given)

Comparing that method with the one from the original tocmacro showed, there was an additional parameter introduced with NTOC: the second parameter formatter is not to be found with the original macro.

At this place I have to give up - I have no further ideas, as I am not that deep in Python. I hope these notes are helpful for you at least...

Changed 6 years ago by izzy

Patch on macros.py for help text correction and disabling version check

comment:4 Changed 6 years ago by izzy

Since I'm just on it, I applied a little patch for the descriptions on the WikiMacros page, disabling some CamelCase stuff which is not intended to resolve to links (so with this patch, it does this no longer).

Well, the patch also disables the version check - the reason is described in comment:3.

comment:5 Changed 6 years ago by izzy

One more remark: The update to __init.py described in comment:3 doesn't seem to be required. Being a bit confused this was missing with tocmacro, too, I removed it and built the egg again. Macro still shows up (admin page and WikiMacros - and thanx to the patch, the phrases SubPage etc. are no longer CamelCase) - but the problem with the 3rd parameter remains.

Still didn't gave up, luckily. Starting to become a Python coder. Got it working - applying the patch as soon as I submitted this post. The trick was to simply remove the formatter as argument, and then replace all occurences of formatter with self.formatter. Macro now shows up where it should, and even works ;)

So the only thing left seems to be the version check. With my patch, it is simply disabled now - so the patch should only be applied on the 0.11 branch (and since there is no such branch, it should only be applied by those who run 0.11).

Changed 6 years ago by izzy

Cumulative patch (includes helptext.patch) to finally make the [[NTOC]] macro work

Changed 6 years ago by izzy

Cumulative patch (including the previous two), making [[TOC]] an alias to [[NTOC]] (as described in comment:1)

comment:6 Changed 6 years ago by izzy

I just added the full patch (including overwriting the [[TOC]] macro, as described in comment:1) as well (macros.py.replace_toc.patch). As each of the macros has either to be enabled manually, it cannot hurt to have it in the code (maybe some "help text" should be applied to point to this). Tested and approved, it works for me this way.

comment:7 Changed 6 years ago by gotoh

Replying multiple comments:

  1. version check issue: I've forgot to mention about it on my comment. 0.11 is required. I'll fix code for version check.
  2. dependency: NTOC depends on TOC macro and it should be enabled. If not, NTOC is not decleared and not showed on admin page. I can add dependency on setup.py and force install TOC macro, but I don't because wiki negotiator itself does not depends on it.
  3. get_page_text() argument issue: 2nd parameter 'formatter' is exist in TOC macro in r4366 (see source tocmacro/0.11/tractoc/macro.py@4366#L199). Please check version of your TOC macro or use latest NTOC code (r4418) which support old style).
  4. helptext.patch: Thanks, I'll apply it.
  5. macros.py.patch: I think formatter issue should be solved if you get latest tocmacro code.
  6. wildcard: I have an idea but not yet implemented.

Any way, I've fixed in r4418. Try it please.

comment:8 Changed 6 years ago by gotoh

  • Resolution fixed deleted
  • Status changed from closed to reopened

(reopen for confirmation from izzy and supporting wildcard)

comment:9 Changed 6 years ago by gotoh

  • Type changed from defect to enhancement

Wildcard is supported in r4422 and now it works with trac 0.12dev (trunk).

Izzy, please test with recent code (ver. 1.4, r4422) and close this issue
if you get success.

NOTE: redefining TOC macro is not enabled by default. I've put it as comment out'ed code
on the tail of the macro.py. Please uncomment if you want.

comment:10 Changed 6 years ago by izzy

Unfortunately no success. I luckily tried the code only in my VMWare test instance:

  • updated tocmacro to latest SVN code
  • updated wikinegotiator to latest code (still needs a patch: Your "Bonus" code should go inside the try..except block, or it would cause some Error 500 if the preceding block fails)
  • enabled both, NTOC and TOC
  • Checked one page having a [[TOC]] on, got:
    Error: Macro TOC(None) failed
    
       maximum recursion depth exceeded
    
  • disabled the [[TOC]] overloader, to verify which code causes this: [[TOC]] loads fine

Conclusion: It must be the [[NTOC]] macro screwed it up. To verify once more, I replaced the [[TOC]] with [[NTOC]], and got:

Error: Macro NTOC(None) failed

  unbound method expand_macro() must be called with TOCMacro instance as first argument (got NTOCMacro instance instead)

So the "max recursion depth" possibly comes by a loop: TOC calls NTOC which fails and calls TOC which calls NTOC...

Guess you screwed it up :-0

Test page is just the SandBox as shipped with Trac, having two 2nd level headers (== John Doe ==) added so a TOC could be created.

Waiting for your update. I can test today only for the next 5 hours, and will not be available tomorrow (just to let you know that you don't wonder if there is no reaction from me in that time).

comment:11 Changed 6 years ago by izzy

There's also something broken with the MultiLangTitleIndex now: If I have e.g. SandBox and SandBox.de, and my default_lang is set to "en", the index before looked like:

SandBox (de, other)

but now only displays

SandBox (de)

I.e. if my browser is set to "de", I have no chance to get to the English page (without knowing I could add ?lang=other to the URL - which you cannot expect from the average user). Maybe you fix this as well? And if you are just at that: Wouldn't it be smarter to display the default_lang instead of "other" - so with the example given, the Index looks like

SandBox (de, en)

?

comment:12 in reply to: ↑ description ; follow-up: Changed 6 years ago by gotoh

Both issue (recursion exceed and 'other' lang) should be fixed in latest code (r4424).
Try it please.

and...

updated wikinegotiator to latest code (still needs a patch: Your "Bonus" code should go inside the try..except block, or it would cause some Error 500 if the preceding block fails)

I didn't placed it in try...except block because I worried that
user may wrong indent level (count of space).
This is temporary style and I want to make this feature configurable without modifying code.

comment:13 in reply to: ↑ 12 Changed 6 years ago by izzy

Replying to gotoh:

Both issue (recursion exceed and 'other' lang) should be fixed in latest code (r4424).
Try it please.

Perfect! Works without any problems now (except that it still displays "other" instead of $default_lang - but that can be counted as a separate RFE if you like (just tell me if I should open a separate ticket for that - maybe I'll even attach the patch for it then)

updated wikinegotiator to latest code (still needs a patch: Your "Bonus" code should go inside the try..except block, or it would cause some Error 500 if the preceding block fails)

I didn't placed it in try...except block because I worried that
user may wrong indent level (count of space).
This is temporary style and I want to make this feature configurable without modifying code.

Well, so why comment it out anyway? It either has to be explicitely enabled. If you can place any comment to be displayed along it in WebAdmin - and also in the Wiki describing this option, it should be fine.

So you may go ahead and close this ticket (if you want the "other" stuff using a separate RFE). Thank you very much for your fast responses and implementation!

Changed 6 years ago by izzy

Using default_lang instead of 'other' in MultiLangTitleIndex? (Patch against r4424)

comment:14 Changed 6 years ago by izzy

As you can see: I did the "other" stuff myself meanwhile - feel free to apply the patch. Seems to work for me - but as I said: My Python programming is mostly cut'n'paste, so you might want to re-check.

Note: 'other' is still displayed if default_lang is empty (just in case).

comment:15 Changed 6 years ago by izzy

Another comment: I know why you put the overload for TOCMacro at the end and commented it out. The better solution would be to put it into the preceding try..catch block and leave it activated. Even if someone does not want to use it, this is no problem since it either has to be activated in trac.ini. This way you catch two problems at once: Noone has to tamper with the code anymore, and it does not cause an exception when TOCMacro fails (which would be the case if NTOCMacro fails silently).

By adding my latest patch and considering what I wrote in this comment, the ticket could be closed. This solution has been approved by me on multiple trac environments and servers, and works as expected.

comment:16 Changed 6 years ago by gotoh

  • Resolution set to fixed
  • Status changed from reopened to closed

As you suggested, implemented in r4974.
Now the macros TitleIndex and TOC are automatically replaced.

comment:17 Changed 6 years ago by izzy

Maybe you forgot to remove the "old override" (commented out) at the end? Is that obsolete now? I see the other (similar declaration), so I assume it is obsolete. To avoid confusion, maybe you want to remove the commented-out part at the end?

comment:18 Changed 6 years ago by gotoh

Thanks, I've forgotten.
That comment is removed in r4978.

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.