Make application/julia a text MIME #18441

TotalVerb · 2016-09-10T23:39:06Z

Add application/julia as a text MIME, and sort the list of text MIMEs.

I wonder if istextmime could be made a little more intelligent. All mime types with text should be text MIMEs, right?

stevengj · 2016-09-11T01:03:08Z

istextmime should return true for anything that should be represented by a String (as opposed to raw binary data), and as far as I can tell this is true for anything in text/ ... indeed, according to RFC6557, which seems to be routinely ignored today thanks to UTF-8, the default charset of text/* data is ASCII.

(I personally hate the fact that programming languages are supposedly in the "application" namespace and not in "text". But IANA does not seem particularly consistent here.... see text/ecmascript vs application/javascript, and those are basically the same language!)

TotalVerb · 2016-09-11T01:53:17Z

Note that text/ecmascript is marked OBSOLETE while application/ecmascript is COMMON. Furthermore, the quoted text there is listed as being considered outdated and having been replaced with UTF-8.

TotalVerb · 2016-09-11T01:59:19Z

I suspect this definition would work as well:

istextmime{T}(::MIME{T}) = startswith(string(T), "text/") || T in [... list of other mimes ...]

Constant-folding during specialization should make performance equivalent to presently; if that does not happen then I suspect marking some things Base.@pure will work.

TotalVerb · 2016-09-11T02:02:10Z

The @textmime macro can be retained in the event a package needs to monkey-patch a text MIME in, although I don't think this is considered a public API.

nalimilan · 2016-09-11T08:43:20Z

FWIW, the Freedesktop.org shared MIME database has a <sub-class-of type="text/plain"/> attribute identifying text files. Can be useful as a reference, in particular if we want to ensure before each release that we haven't missed new MIME types (e.g. for new languages).

tkelman · 2016-09-11T20:19:35Z

base/multimedia.jl

@@ -81,7 +81,7 @@ istextmime(m::AbstractString) = istextmime(MIME(m))
 reprmime(m::AbstractString, x) = reprmime(MIME(m), x)
 stringmime(m::AbstractString, x) = stringmime(MIME(m), x)

-for mime in ["text/vnd.graphviz", "text/latex", "text/calendar", "text/n3", "text/richtext", "text/x-setext", "text/sgml", "text/tab-separated-values", "text/x-vcalendar", "text/x-vcard", "text/cmd", "text/css", "text/csv", "text/html", "text/javascript", "text/markdown", "text/plain", "text/vcard", "text/xml", "application/atom+xml", "application/ecmascript", "application/json", "application/rdf+xml", "application/rss+xml", "application/xml-dtd", "application/postscript", "image/svg+xml", "application/x-latex", "application/xhtml+xml", "application/javascript", "application/xml", "model/x3d+xml", "model/x3d+vrml", "model/vrml"]
+for mime in ["application/atom+xml", "application/ecmascript", "application/javascript", "application/julia", "application/json", "application/postscript", "application/rdf+xml", "application/rss+xml", "application/x-latex", "application/xhtml+xml", "application/xml", "application/xml-dtd", "image/svg+xml", "model/vrml", "model/x3d+vrml", "model/x3d+xml", "text/calendar", "text/cmd", "text/css", "text/csv", "text/html", "text/javascript", "text/latex", "text/markdown", "text/n3", "text/plain", "text/richtext", "text/sgml", "text/tab-separated-values", "text/vcard", "text/vnd.graphviz", "text/x-setext", "text/x-vcalendar", "text/x-vcard", "text/xml"]


should line wrap this while we're at it

Is there a standard way to wrap long for loops? I just pushed to the beginning of line here.

I personally follow the matlab convention of indenting line continuations of block openers one level deeper than the body of the block

TotalVerb · 2016-09-13T04:42:05Z

There seems to be demand for a more advanced change. I'm playing around with turning istextmime into a trait, and then dispatching on this trait instead of the clunky @textmime macro. Unfortunately, returning a Bool is currently not good enough (this is essentially #17880), because the return type is known to be a constant too late, and the type instability in reprmime cannot be removed (though LLVM can remove the dead code).

So the new change implements istextmime using a lower-level mimetypetype trait (name could/should be changed to something more descriptive, but I couldn't figure out what). It turns out that this actually simplifies the code quite a bit at this point. While I was working on this, I moved docstrings over from helpdb.jl. In fact, these docstrings mostly duplicated the big comment near the top about text vs. binary mimes.

Remaining issues:

Do I need to deprecate @textmime (as I've done here), or should I just rip it out? There exist usages in the wild.
What (if anything) should be exported?
Does this pessimize performance at all? I wonder if nanosoldier has multimedia benchmarks.

TotalVerb · 2016-09-13T04:48:49Z

Hmm, it looks like the trait did not avoid the ::Union{Array{UInt8,1},String} pessimization of reprmime's return type. I guess the implementation of istextmime might be too complicated. Will try a @pure annotation.

TotalVerb · 2016-09-13T04:58:26Z

The compiler really doesn't like the complexity of the mimetypetype function; adding Base.@pure did not seem to help. My suspicion is that stringmime and reprmime are fairly infrequently used in performance-critical situations, and that when they are the cost of allocation would dominate the cost of the type instability. Perhaps this performance issue can be tolerated until #14324 can be implemented.

Alternatively, we can put the text/ mime types back into the list to maintain.

tkelman · 2016-09-13T05:09:31Z

base/multimedia.jl

 macro textmime(mime)
+    Base.depwarn(string("`@textmime mime` is deprecated; use ",


if deprecated, it should probably move to deprecated.jl

It needs to be in the Multimedia module though.

ah, is it not exported from there? could be eval'ed into that module

TotalVerb · 2016-09-14T02:14:09Z

I doubt the performance is an issue, so I think this should be good to go now?

stevengj · 2016-09-14T02:24:08Z

I guess so. I have to be honest that I'm not a huge fan of the whole traits technique in cases like this; it seems like an overly complicated way to determine something at compile time that is unlikely to ever be performance-sensitive.

TotalVerb · 2016-09-14T02:29:30Z

@stevengj Well, due to inference limitations these traits aren't even working properly. (We need #14324 to remove the type instability with traits, and with booleans we need #17880). I'm happy to do this with booleans if people like that better.

TotalVerb · 2016-09-14T02:31:04Z

In fact, the original approach was with booleans; then I switched to traits because I ran into #17880, and then I discovered that the functions was too complex for even traits to fix the type instability...

stevengj · 2016-09-14T03:17:51Z

Speaking for myself, I'd prefer booleans.

TotalVerb · 2016-09-14T03:24:11Z

@stevengj Just pushed a boolean version. The boolean version is far simpler too, and probably also has lower compilation overhead. I like it also.

TotalVerb · 2016-09-14T03:24:49Z

Oops, I need to fix the deprecated @textmime macro too. Is there a file to test deprecated stuff, or is this generally not done?

stevengj · 2016-09-14T14:10:07Z

I don't think we generally add tests for deprecations.

stevengj · 2016-09-14T14:12:52Z

base/deprecated.jl

+# Deprecate @textmime into the Multimedia module, #18441
+eval(Multimedia, :(macro textmime(mime)
+    Base.depwarn(string("`@textmime mime` is deprecated; use ",
+        "`Base.Multimedia.mimetypetype(::MIME{mime}) = ",


Shouldn't the deprecation now be Base.istextmime(::MIME{mime}) = true?

Oops. The code is correct, but I forgot to update the text.

TotalVerb · 2016-10-03T17:54:00Z

Is this good to squash and merge?

…iaLang#21884)

(cherry picked from commit af92634)

tkelman reviewed Sep 11, 2016
View reviewed changes

TotalVerb force-pushed the fw/text-mime branch from 9b144d8 to c30e797 Compare September 13, 2016 04:42

tkelman reviewed Sep 13, 2016
View reviewed changes

kshyatt added the io Involving the I/O subsystem: libuv, read, write, etc. label Sep 13, 2016

TotalVerb force-pushed the fw/text-mime branch from 18f0153 to b3e1298 Compare September 14, 2016 03:24

stevengj reviewed Sep 14, 2016
View reviewed changes

TotalVerb added 8 commits September 28, 2016 22:07

Make application/julia a text MIME

31c073f

Use trait instead of istextmime

2962288

Add a @pure annotation

34ba989

Run genstdlib

c8ad5dc

Move deprecation to deprecated.jl

7fbc096

Use boolean instead of traits

3d658f2

Fix deprecated @textmime

d05866c

Fix @textmime depwarn message

ee6ee0c

TotalVerb force-pushed the fw/text-mime branch from 6c066c3 to ee6ee0c Compare September 29, 2016 02:11

Undo accidental re-helpdbification

f003f00

stevengj merged commit bc9b6ef into JuliaLang:master Oct 3, 2016

martinholters mentioned this pull request Oct 5, 2016

Couple of small docs fixes #18784

Merged

Sacha0 added a commit to Sacha0/julia that referenced this pull request May 14, 2017

Add NEWS.md entry for at-textmime deprecation (JuliaLang#18441).

20ce296

Sacha0 added deprecation This change introduces or involves a deprecation needs news A NEWS entry is required for this change labels May 14, 2017

tkelman removed the needs news A NEWS entry is required for this change label May 16, 2017

tkelman pushed a commit that referenced this pull request May 16, 2017

Add NEWS.md entry for at-textmime deprecation (#18441). (#21884)

af92634

singam-sanjay pushed a commit to singam-sanjay/julia that referenced this pull request May 16, 2017

Add NEWS.md entry for at-textmime deprecation (JuliaLang#18441). (Jul…

52b03f0

…iaLang#21884)

tkelman pushed a commit that referenced this pull request May 16, 2017

Add NEWS.md entry for at-textmime deprecation (#18441). (#21884)

92c95c0

(cherry picked from commit af92634)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make application/julia a text MIME #18441

Make application/julia a text MIME #18441

TotalVerb commented Sep 10, 2016

stevengj commented Sep 11, 2016

TotalVerb commented Sep 11, 2016 •

edited

Loading

TotalVerb commented Sep 11, 2016 •

edited

Loading

TotalVerb commented Sep 11, 2016

nalimilan commented Sep 11, 2016

tkelman Sep 11, 2016

TotalVerb Sep 13, 2016

tkelman Sep 13, 2016

TotalVerb commented Sep 13, 2016 •

edited

Loading

TotalVerb commented Sep 13, 2016

TotalVerb commented Sep 13, 2016

tkelman Sep 13, 2016

TotalVerb Sep 13, 2016

tkelman Sep 13, 2016

TotalVerb Sep 13, 2016

TotalVerb commented Sep 14, 2016

stevengj commented Sep 14, 2016

TotalVerb commented Sep 14, 2016

TotalVerb commented Sep 14, 2016

stevengj commented Sep 14, 2016

TotalVerb commented Sep 14, 2016

TotalVerb commented Sep 14, 2016 •

edited

Loading

stevengj commented Sep 14, 2016

stevengj Sep 14, 2016

TotalVerb Sep 14, 2016

TotalVerb Sep 14, 2016

TotalVerb commented Oct 3, 2016

		macro textmime(mime)
		Base.depwarn(string("`@textmime mime` is deprecated; use ",

Make application/julia a text MIME #18441

Make application/julia a text MIME #18441

Conversation

TotalVerb commented Sep 10, 2016

stevengj commented Sep 11, 2016

TotalVerb commented Sep 11, 2016 • edited Loading

TotalVerb commented Sep 11, 2016 • edited Loading

TotalVerb commented Sep 11, 2016

nalimilan commented Sep 11, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TotalVerb commented Sep 13, 2016 • edited Loading

TotalVerb commented Sep 13, 2016

TotalVerb commented Sep 13, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TotalVerb commented Sep 14, 2016

stevengj commented Sep 14, 2016

TotalVerb commented Sep 14, 2016

TotalVerb commented Sep 14, 2016

stevengj commented Sep 14, 2016

TotalVerb commented Sep 14, 2016

TotalVerb commented Sep 14, 2016 • edited Loading

stevengj commented Sep 14, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TotalVerb commented Oct 3, 2016

TotalVerb commented Sep 11, 2016 •

edited

Loading

TotalVerb commented Sep 11, 2016 •

edited

Loading

TotalVerb commented Sep 13, 2016 •

edited

Loading

TotalVerb commented Sep 14, 2016 •

edited

Loading