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

Permit using string macro with "qualified" name #18690

Merged
merged 5 commits into from
Dec 7, 2016

Conversation

TotalVerb
Copy link
Contributor

@JeffBezanson raised some concerns in #14208 (comment), but I think that overall this change is fairly intuitive. Being able to call string macros that happen to have the same name in different modules is a big plus in my opinion.

Close #6970.
Close #14208.

@TotalVerb
Copy link
Contributor Author

The segmentation fault is strange. Is this a known issue with the nightly?

@kshyatt
Copy link
Contributor

kshyatt commented Sep 28, 2016

I restarted your build, @TotalVerb. If it succeeds, are we ready to merge this?

@kshyatt
Copy link
Contributor

kshyatt commented Sep 28, 2016

@JeffBezanson or @StefanKarpinski thoughts?

@JeffBezanson JeffBezanson added the needs news A NEWS entry is required for this change label Sep 28, 2016
@JeffBezanson
Copy link
Member

LGTM. Should probably have a NEWS entry and/or doc mention.

@kshyatt kshyatt added the needs docs Documentation for this change is required label Sep 28, 2016
@TotalVerb
Copy link
Contributor Author

Added NEWS and doc entries.

@TotalVerb
Copy link
Contributor Author

TotalVerb commented Sep 29, 2016

This is somewhat orthogonal to this PR, but the documentation on "non-standard string literals" does not mention the commonly-used synonym "string macro". Should it?

@@ -24,6 +24,14 @@ Language changes
This can be changed back to the original color by setting the environment variable `JULIA_INFO_COLOR` to `"blue"`.
One way of doing this is by adding `ENV["JULIA_INFO_COLOR"] = :blue` to the `.juliarc.jl` file.
For more information regarding customizing colors in the REPL, see this [manual section]( http://docs.julialang.org/en/latest/manual/interacting-with-julia/#customizing-colors).
* Multiline and singleline command macros have been added. A command macro is
like a string macro, but the syntax uses backquotes (<code>`</code>)
Copy link
Contributor

Choose a reason for hiding this comment

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

does this work in markdown? it's confusing the github rendering

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ugh, markdown makes it really difficult to type backquotes in code. I think I need to escape it like `. Unless anyone has a better solution?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated with the backslashes and it looks like it's rendering OK:

Copy link
Contributor

Choose a reason for hiding this comment

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

I sometimes do ``` which is "triple-backtick space single-backtick space triple-backtick" but whatever works

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting; apparently there's no physical limit to how many backticks can be used. ``` works as well for encoding triple backticks. In general, I guess it suffices to delimit with one more consecutive backtick than will be required in the text.

But I'll probably just leave this one be, since it seems to work.

@TotalVerb
Copy link
Contributor Author

Can someone restart the Travis build again?

@tkelman
Copy link
Contributor

tkelman commented Oct 3, 2016

Needs a rebase apparently, so that'll restart it anyway.

@TotalVerb
Copy link
Contributor Author

Rebased and fixed the <code> stuff.

@tkelman tkelman removed the needs docs Documentation for this change is required label Oct 3, 2016
@simonbyrne simonbyrne added this to the 0.6.0 milestone Dec 7, 2016
@simonbyrne
Copy link
Contributor

Would you mind fixing the conflicts? Then I think we can merge this.

@TotalVerb
Copy link
Contributor Author

Rebased.

``` q`xyz` ``` is equivalent to `@q_cmd "xyz"`.
* Nonstandard string and command literals can now be qualified with their
module. For instance, `Base.r"x"` is now parsed as `Base.@r_str "x"`.
Previously, this syntax parsed as an implicit multiplication.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a link to the PR: add ([#18690]) here, and a link at the bottom.

nonstandard command literal is like a nonstandard string literal, but the
syntax uses backquotes (``` ` ```) instead of double quotes, and the
resulting macro called is suffixed with `_cmd`. For instance, the syntax
``` q`xyz` ``` is equivalent to `@q_cmd "xyz"`.
Copy link
Contributor

Choose a reason for hiding this comment

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

And a link here, if you can find the PR (I can't).

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, it was yours: #18644

@simonbyrne
Copy link
Contributor

Nevermind, turns out I could do it.

@simonbyrne simonbyrne merged commit ed7dfe4 into JuliaLang:master Dec 7, 2016
@MichaelHatherly
Copy link
Member

make check-whitespace is failing on this.

@simonbyrne
Copy link
Contributor

Blegh, my bad!

@simonbyrne
Copy link
Contributor

simonbyrne commented Dec 7, 2016

Sorry, I thought editing the NEWS.md would be harmless; I was wrong. Fixed now.

@MichaelHatherly
Copy link
Member

👍 no prob, thanks for the quick fix.

@Sacha0 Sacha0 removed the needs news A NEWS entry is required for this change label May 12, 2017
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.

7 participants