Edgewall Software
Modify

Opened 8 years ago

Closed 8 years ago

Last modified 4 years ago

#9915 closed enhancement (fixed)

Extend RST to support additional directive types

Reported by: anatoly techtonik <techtonik@…> Owned by: Christian Boos
Priority: high Milestone: 0.12.2
Component: rendering Version:
Severity: normal Keywords: rst directive role
Cc: Branch:
Release Notes:

Fix a potential race condition in reStructuredText rendering

API Changes:

Registered reStructuredText roles and directives can gain access to the Trac environment and rendering context corresponding to the docutils content being processed.

Description

When Trac shows WikiRestructuredText page from Sphinx for example it shows a lot of "Unknown directive type" errors - http://www.google.com/search?q=trac+Unknown+directive+type

It would be nice if installed Sphinx (and other packages) could provide these directives for Trac.

Attachments (1)

9915-register-once-r10420.diff (13.7 KB ) - added by Christian Boos 8 years ago.
rst: don't store Trac env and context in the directive and role handlers.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 8 years ago by Christian Boos

I'm not sure if "running" those directives in isolation would make sense… what do you think a .. currentmodule: blah directive should do?

OTOH, showing big scary red boxes for such missing directives is indeed very annoying, but this has already been taken care of in trunk (r10358). See e.g. source:trunk/doc/api/trac_env.rst.

"worksforme"?

comment:2 Changed 8 years ago by anatoly techtonik <techtonik@…>

The target page seem much better. But..

It would be nice to see at least Sphinx integration. For example, cross-link functions referenced in the same page. Perhaps these question marks popups could link to documentation how to provide Macros/Extensions to make those unknown items familiar to Trac. Then Sphinx project (or smb. else) could release an extension at least. Is that possible in current state?

comment:3 Changed 8 years ago by Christian Boos

Keywords: rst directive role added
Milestone: next-minor-0.12.x
Priority: normalhigh

The trac.mimeview.rst.ReStructuredTextRenderer.render() method could take care of saving the current Trac environment and rendering context somewhere in the docutils inputs and this could allow plugins to register docutils directives and roles that could make a safe use of Trac's API by retrieving these informations.

Now that I'm staring at the code… I see that we currently register the trac directive and role each time in render(), the component (self) and context being free variables there, picked from the render() method, and this is completely thread-unsafe.

The trac directive and role should be registered once, and should retrieve the env and context through the docutils inputs, like proposed above. This input should probably be a custom Inliner, as this will be available from both the role (directly) and the directive (through state_machine.inliner).

Finally, the trac role should be a "canonical" one even, no need to localize the "trac" name.

Changed 8 years ago by Christian Boos

rst: don't store Trac env and context in the directive and role handlers.

comment:4 Changed 8 years ago by Christian Boos

API Changes: modified (diff)
Milestone: next-minor-0.12.x0.12.2
Owner: set to Christian Boos
Release Notes: modified (diff)
Status: newassigned

9915-register-once-r10420.diff implements the approach described in comment:3 (except that inliner should be retrieved from the state instead of the state_machine, as I misread the docutils code).

comment:5 Changed 8 years ago by anatoly techtonik <techtonik@…>

Unfortunately I am not the one who is able to review this diff, but I am glad it is given a go. Perhaps I need more time or an example plugin to see how it works.

comment:6 Changed 8 years ago by Christian Boos

One thing I didn't test is the compatibility with older docutils (tested 0.7). We still have "docutils≥0.3" as the requirement in setup.py, not sure if Trac still works with 0.3 though (the TracInstall page even says docutils version ≥ 0.3.9).

comment:7 Changed 8 years ago by Christian Boos

I've tested the patch (+ minor modifications) with docutils 0.3.9, 0.4, 0.5, 0.6 and 0.7. Everything seems to work fine. Applied in r10458.

comment:8 Changed 8 years ago by Christian Boos

Resolution: fixed
Status: assignedclosed

comment:9 in reply to:  6 Changed 4 years ago by Ryan J Ollos

Replying to cboos:

One thing I didn't test is the compatibility with older docutils (tested 0.7). We still have "docutils≥0.3" as the requirement in setup.py, not sure if Trac still works with 0.3 though (the TracInstall page even says docutils version ≥ 0.3.9).

docutils version requirement in setup.py set to 0.3.9 in [13081:13083] (comment:7:ticket:11694).

Last edited 4 years ago by Ryan J Ollos (previous) (diff)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Christian Boos.
The resolution will be deleted.
to The owner will be changed from Christian Boos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.