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

[WIP] Updated json string literal based on elcritch's work #130

Closed
wants to merge 2 commits into from

Conversation

tshort
Copy link

@tshort tshort commented Nov 28, 2015

The aim of this PR is to update and simplify @elcritch's PR #113.

The single-quote option is removed. There's also no option to pass the dicttype. I tried using a trailing argument for this, but I couldn't find an eval option that worked.

It's marked as WIP because there's some extra junk in there for my failed tries to get dicttype to work. Someone else may have a suggestion. In any case, it needs to be cleaned up.

@tshort
Copy link
Author

tshort commented Dec 15, 2015

Any comments on this? Objections to merging?

The test failures are on Julia master and don't relate to this PR.

@elcritch
Copy link

I'll be able to take a look at it soon. Hopefully tomorrow. 

On Mon, Dec 14, 2015 at 6:13 PM -0800, "Tom Short" [email protected] wrote:

Any comments on this? Objections to merging?

The test failures are on Julia master and don't relate to this PR.


Reply to this email directly or view it on GitHub.

@@ -289,4 +289,17 @@ function parsefile{T<:Associative}(filename::AbstractString; dicttype::Type{T}=D
end
end

# Macros
str_helper(arg) = parse(arg)
Copy link
Collaborator

@TotalVerb TotalVerb Jun 11, 2016

Choose a reason for hiding this comment

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

this function can probably be taken out, parse can be used directly

@TotalVerb
Copy link
Collaborator

TotalVerb commented Jun 11, 2016

As is, I'm not convinced this is useful—it doesn't save much typing over JSON.parse. It also boils down to macro json_str(x) JSON.parse(x) end which someone can trivially implement if needed (say, for avoiding runtime cost... though deferring the cost to compile time doesn't seem any better).

But one feature that would be nice would be interpolation:

const x = "World"
json"""
{
    "Hello": $x
}
"""

to produce Dict("Hello" => "World").

This interpolation feature is supported by Cxx.jl, MATLAB.jl, and a similar feature has been experimentally implemented in Scala.


let mstr_test = @compat Dict("a" => 1)
@test mstr_test == json"""{"a":1}"""
# Doesn't work:
Copy link
Collaborator

@TotalVerb TotalVerb Jun 11, 2016

Choose a reason for hiding this comment

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

remove the test instead of commenting it out?

@elcritch
Copy link

@TotalVerb having string interpolation in the string macro would be very useful, IMHO. Unfortunately I couldn't find any documentation on how string interpolation is done by on normal Julia parse/compile. It appears you either get string interpolation or you can do string macros.

@StefanKarpinski
Copy link
Member

StefanKarpinski commented Jun 16, 2016

String interpolation can't be done in full generality in a string macro since the parser needs to call itself recursively in order to know when the string literal ends. You can approximate it, however, by looking for $ and using the parse function to find an expression. This will not support interpolation of string literals.

@TotalVerb
Copy link
Collaborator

TotalVerb commented Aug 19, 2016

@StefanKarpinski While what you say is entirely correct, this macro is typically used multiline, so string interpolation with string literals (as long as they are not also multiline) should work as expected. I hope to create a prototype soon.

@TotalVerb
Copy link
Collaborator

As I mentioned here, this macro doesn't really provide anything beyond JSON.parse("..."). A different package would be a better place for a replacement for literal dict syntax.

@TotalVerb TotalVerb closed this Apr 25, 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.

4 participants