Edgewall Software

Opened 17 years ago

Closed 12 years ago

Last modified 10 years ago

#5533 closed enhancement (fixed)

Add support for additional mime types using pygments get_all_lexers — at Version 20

Reported by: loop Owned by: Christian Boos
Priority: normal Milestone: 1.0
Component: rendering Version: devel
Severity: major Keywords: actionscript pygments syntax coloring
Cc: Tim Hatch, lists@…, ryano@…, ethan.jucovy@… Branch:
Release Notes:

If Pygments support is enabled, all mimetypes supported by Pygments become automatically known to Trac

API Changes:
Internal Changes:

Description

i contributed an actionscript lexer to the pygments project. so it would be nice to add it also to trac as default with a following line in api.py :

'text/x-actionscript':      ['as'],

Change History (21)

comment:1 by Christian Boos, 17 years ago

Well, it would be nicer if we could do such things from the pygments renderer automatically, using the information provided by pygments.lexers.get_all_lexers. Related to #3332.

comment:2 by Remy Blank, 16 years ago

Related to #5828.

comment:3 by Christian Boos, 15 years ago

Component: version control/browserrendering
Milestone: 0.11-retriage0.13

comment:4 by Christian Boos, 14 years ago

Milestone: next-major-0.1X0.12
Severity: normalmajor

Tickets #5613, #8570 were closed as duplicates.

We should try to fix this for 0.12.

comment:5 by Christian Boos, 14 years ago

Cc: Tim Hatch added
Milestone: 0.120.12.1

Maybe for 0.12.1.

comment:6 by Christian Boos, 14 years ago

Milestone: 0.12.1next-major-0.1X

comment:7 by Thijs Triemstra <lists@…>, 14 years ago

Cc: lists@… added
Summary: actionscript mime typeAdd support for additional mime types using pygments get_all_lexers

Renaming the ticket because it's not actionscript-specific anymore.

comment:8 by Thijs Triemstra <lists@…>, 14 years ago

#7847 was closed as a duplicate.

comment:9 by Christian Boos, 12 years ago

#10725 was closed as duplicate.

Time to do something about this bug ;-)

comment:10 by Ryan J Ollos <ryano@…>, 12 years ago

Cc: ryano@… added

comment:11 by Ethan Jucovy <ethan.jucovy@…>, 12 years ago

Cc: ethan.jucovy@… added

by Peter Suter, 12 years ago

Adds pygments.lexers.get_all_lexers info to MIME_MAP

comment:12 by Peter Suter, 12 years ago

Patch attachment:T5533_pygments_lexer_mimetypes.diff​ seems simple enough, am I missing something here?

in reply to:  12 comment:13 by Christian Boos, 12 years ago

Replying to psuter:

Patch attachment:T5533_pygments_lexer_mimetypes.diff​ seems simple enough, am I missing something here?

There's a possibility to miss an initial lookup with this approach…

A Mimeview.get_mimetype() may happen first, for a repository file or an attachment, and the Pygments.get_quality_ratio(mimetype) call happens then, after the mimetype has been (incorrectly) detected.

A refresh (forced refresh for an attachment) will do… but still, maybe we can think about an improved solution.

comment:14 by Christian Boos, 12 years ago

For example:

  • [72aed033/cboos.git] 1.0dev: add IHTMLPreviewRenderer.get_extra_mimetypes method for augmenting the known mimetypes.

comment:15 by Christian Boos, 12 years ago

Milestone: next-major-releases1.0

Ok with my proposed fix?

comment:16 by Remy Blank, 12 years ago

Looks good, yes.

comment:17 by Peter Suter, 12 years ago

Sorry, I meant to check this. Yes, looks good.

The comment describing the new optional get_extra_mimetypes() method on the IHTMLPreviewRenderer interface could maybe be slightly improved. Currently it says Returning nothing is fine, but return None would not work.

More generally, how would a plugin know what the already known mime types are? Should it care? What about conflicts, priorities etc.? (Maybe out of scope for now.)

Also it doesn't really describe what the effect will be of adding additional mimetypes, and why this is related to HTML preview rendering.

Or maybe these details fit better in TracDev instead.

Targeting milestone 1.0 seems a bit risky for a plugin interface addition.

in reply to:  17 comment:18 by Christian Boos, 12 years ago

Replying to psuter:

Sorry, I meant to check this. Yes, looks good.

The comment describing the new optional get_extra_mimetypes() method on the IHTMLPreviewRenderer interface could maybe be slightly improved. Currently it says Returning nothing is fine, but return None would not work.

Ah yes, forgot the usual "... or []" when iterating on providers.

More generally, how would a plugin know what the already known mime types are? Should it care? What about conflicts, priorities etc.? (Maybe out of scope for now.)

Well, if that's really a concern (i.e. a plugin wants only to add some types and not override them), then it can simply test via Mimeview(self.env).get_mimetype('file.keyword'). If that's not enough and access to that component's _mime_map is needed, we should indeed fix that with a better API (#3332 or something like that…).

Also it doesn't really describe what the effect will be of adding additional mimetypes, and why this is related to HTML preview rendering.

I've edited the docstring in this direction. It's true it has little to do with "HTML preview", except for the fact that IHTMLPreviewRenderer is what PygmentsRenderer implements and that IHTMLPreviewRenderer is already flagged as "to be merged in IContentConverter with an improved API", so this seems the best interim solution.

Or maybe these details fit better in TracDev instead.

Targeting milestone 1.0 seems a bit risky for a plugin interface addition.

Well, compared to the benefits this feature provides, I think this implementation is "good enough" and worth including in 1.0. I wanted to implement it since a very long time, and always failed because I didn't find a "clean" enough way to do it (short of bringing the whole #3332 topic on the table again). Your simple approach with add_mimetype motivated me to find a middle ground.

comment:19 by Christian Boos, 12 years ago

Resolution: fixed
Status: newclosed

comment:20 by Christian Boos, 12 years ago

Release Notes: modified (diff)
Note: See TracTickets for help on using tickets.