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

Fix macro name for @sprintf errors #6200

Merged
merged 2 commits into from
Mar 22, 2014
Merged

Fix macro name for @sprintf errors #6200

merged 2 commits into from
Mar 22, 2014

Conversation

simonster
Copy link
Member

This is a bit pedantic, but before we'd get:

julia> @sprintf "%d"
ERROR: @printf: wrong number of arguments

Also fix triggering condition for "format must be a plain static string"
error
@simonster
Copy link
Member Author

Okay, updated to get the errors right in all conditions (I think!). I also fixed is_str_expr so that it actually triggers. (I suspect there have been some changes to AST structure since this was written.)

@ivarne
Copy link
Member

ivarne commented Mar 19, 2014

See also #5847

@simonster
Copy link
Member Author

Now includes a further fix: @sprintf notaformatstring now correctly prints "@sprintf: first argument must be a format string" instead of "@printf: first or second argument must be a format string".

@StefanKarpinski
Copy link
Member

Man, I feel for GitHub users @printf and @sprintf...

@simonster
Copy link
Member Author

Whoops, sorry guys.

@StefanKarpinski
Copy link
Member

Testing now, but this all looks good to me.

StefanKarpinski added a commit that referenced this pull request Mar 22, 2014
@StefanKarpinski StefanKarpinski merged commit 898962c into master Mar 22, 2014
@StefanKarpinski StefanKarpinski deleted the sjk/sprintf branch March 22, 2014 00:19
@StefanKarpinski
Copy link
Member

Thanks for fixing all that. We are not meticulously apt in our error messaging :-)

@simonster
Copy link
Member Author

Thanks for reviewing!

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