Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename Julia's jldoctest alias #1511

Merged
merged 2 commits into from
May 21, 2017
Merged

Conversation

mortenpi
Copy link
Contributor

@mortenpi mortenpi commented May 3, 2017

The alias should really refer to the special syntax of writing REPL snippets, e.g.

julia> julia.code()
non-julia output

Something being a "doctest" does not specify how it should be highlighted. In fact, doctests can be both ordinary code snippets (i.e. should get language-julia class) and REPL snippets (i.e. julia.console class).

Ideally there would be separate highlighter for the julia.console, but just applying the ordinary julia highlighting works well enough that it is fine to have this as an alias.


I'm not entirely sure what the naming convention is for "subsyntaxes" (e.g. julia.console, julia-console or something else?) There are a few existing highlighters out there for the REPL syntax, e.g. Atom's plugin has julia-console and Pygments has jlcon. Overall, spelling the name out is probably better, so it would be best to be consistent with Atom I'd say.

cc @ararslan, x-ref: JuliaDocs/Documenter.jl#465

@@ -84,7 +84,7 @@ function(hljs) {

// placeholder for recursive self-reference
var DEFAULT = {
aliases: ['jldoctest'],
aliases: ['julia.console'],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably better to do julia-console if we're being consistent with Atom

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd tend to agree, and also class: language-julia-console feels better than class: language-julia.console (if I interpret this correctly), but looking around in highlight.js it seems that . is generally used for "scoping" related syntaxes together.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but looking around in highlight.js it seems that . is generally used for "scoping" related syntaxes together.

Huh? I wonder where did you find anything looking remotely like that :-) I'm certainly not aware of such a feature!

Copy link
Contributor Author

@mortenpi mortenpi May 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't mean it's a programmed feature, just a convention 😄 Examples that I can find:

  • pf.js has the pf.conf alias.
  • handlebars has the html.hbs and html.handlebars aliases.
  • Objective-C sets a precedent for using dashes in names and not for scoping (obj-c is one of the aliases).

However, there is also closure / closure-repl, erlang / erlang-repl and vbscript / vbscript-html, which I missed before. Perhaps we should go with julia-repl then?

There's also one side note regarding Atom -- while the grammar is called julia-console, you can't really use it for code blocks in Markdown, or at least I don't know how to select it (neither julia-console nor julia.console seem to work, although it's possible to use julia.console in Markdown preview if the JuliaEditorSupport/atom-language-julia#106 patch is applied). TL;DR -- I don't think being consistent with Atom is a very high priority and we could probably just change the name of the grammar there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@isagalaev, how would one actually use an alias with a period in it? It seems that doing e.g. class="language-julia.console actually just picks julia, ignoring everything after the period (e.g. if I create an alias juliax.console I can't select it at all).

@ararslan
Copy link
Contributor

ararslan commented May 3, 2017

Won't this mean that everyone who currently uses jldoctest, including the Base documentation, will need to update? Seems quite a breaking change. Also cc @MichaelHatherly

@mortenpi
Copy link
Contributor Author

mortenpi commented May 4, 2017

All Documenter builds bundle a particular highlight.js version anyway (v9.5.0 currently), so releasing a new highlight.js version will not fix old builds. Only way to fix old builds would be to have everyone re-run all the tags etc. with a new Documenter version.

@mortenpi
Copy link
Contributor Author

mortenpi commented May 6, 2017

I just realized that I might have been a bit Documenter-specific in my thinking. It might make sense to have proper highlighting also when showing unprocessed jldoctest blocks, e.g. when people run some other Markdown processor, such as some online code preview, and end up with something like language-jldoctest.

I.e. perhaps we should have julia-repl in addition to jldoctest. A proper highlighter for jldoctest should be even more complex though, by having to detect whether it is a repl-type or code-type doctest.

In any case, in Documenter I don't think we should use the jldoctest in the output, since we can easily apply a more appropriate language label ourselves.

@mortenpi
Copy link
Contributor Author

mortenpi commented May 7, 2017

After checking out clojure-repl I saw that it's actually quite easy to implement a simple "superlanguage", hence I think we have a proper julia-repl implementation now (additional testing welcome though).

I also made jldoctest an alias of that since most doctests are REPL-type I believe. It's probably not worth supporting a separate grammar for it and, as I mentioned already, I propose we just rename jldoctest blocks in Documenter.

Allows the highlighting of the following kind of code blocks:

  julia> foo() do x
           bar(x)
         end
  output

Although script-like doctest are supported as well, most doctests in
Julia docs are REPL-type, i.e. look as above. Hence it makes sense to
have jldoctest to be an alias of julia-repl.

Inspired by the clojure-repl implementation.
@mortenpi
Copy link
Contributor Author

Rebase, cleaned up the commits, and also noticed a tiny typo in CHANGES.md. Is there anything specific holding back the merge? @ararslan, do you have a final verdict on this approach for the doctest highlighting?

@ararslan
Copy link
Contributor

I like it! I was wondering though, do you think it'd be worth highlighting error, warning, and info messages in the output rather than no highlighting at all? Or does that seem unnecessary? The immediate problem I see with that is that it's hard to know where exactly a message ends, so the best we could do in terms of highlighting would likely be to just highlight all text on the line that contains ERROR:, WARNING:, INFO:, etc.

@isagalaev isagalaev merged commit acd8fd8 into highlightjs:master May 21, 2017
@isagalaev
Copy link
Member

Merged! Thanks for taking care of the whole process without me having to do anything :-) And yes, a "-repl" language is indeed the best way of handling something like this.

@mortenpi
Copy link
Contributor Author

Thanks @isagalaev!

@ararslan I think that in principle highlighting some of the output could definitely be useful. E.g. I came across an example of @code_warntype for which highlighting the output as Julia code is actually very helpful. ERROR/WARNING/INFO could be another example. But at the same time I'd say you don't want spurious highlighting like we had before, so it could be tricky to get it right.

@mortenpi mortenpi deleted the patch-1 branch May 22, 2017 01:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants