Skip to content

Report printf specifiers from Service API #272

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

Merged
merged 7 commits into from
Nov 23, 2014

Conversation

dungpa
Copy link
Contributor

@dungpa dungpa commented Nov 19, 2014

Close #174.

The PR seems to cover the most important cases:

  • regular vs. verbatim vs. triple-quote strings
  • single-line vs. multiline strings
  • built-in printf functions vs. printf-like user-defined functions

One thing isn't very nice: ASTs don't preserve information whether a string is regular, verbatim or triple-quote. Consequently, we have to lookup the source string to recover that information and create correct offsets for reporting ranges.

@7sharp9
Copy link
Member

7sharp9 commented Nov 19, 2014

So how will this API be used?

Great work by the way, what I wanted for XS was to present format specifiers in completion, thats a different thing to this though.

@dungpa
Copy link
Contributor Author

dungpa commented Nov 19, 2014

@7sharp9
Copy link
Member

7sharp9 commented Nov 19, 2014

Ah, so its a colourisation assist.


if i >= len then failwithf "%s" <| FSComp.SR.forBadPrecision();

let acc = if precisionArg then (Option.map ((+)1) posi, g.int_ty) :: acc else acc
let acc = if precisionArg then (Option.map ((+) 1) posi, g.int_ty) :: acc else acc
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert this kind of change too

@dungpa
Copy link
Contributor Author

dungpa commented Nov 19, 2014

I removed all unnecessary formatting changes (Sorry, I was itching whenever I saw formatting quirks.).

@dsyme
Copy link
Contributor

dsyme commented Nov 19, 2014

This all looks good to me (except remove the laziness)

@dungpa
Copy link
Contributor Author

dungpa commented Nov 20, 2014

Thanks. Laziness is now removed.

@7sharp9
Copy link
Member

7sharp9 commented Nov 20, 2014

So is the aim of this just to have strings and format strings highlighted differently?

@vasily-kirichenko
Copy link
Contributor

This is for highlighting format specifiers in printf family of functions, like this:

image

Currently we use untyped AST + a regex for this in VFPT. But it does not work on user defined functions (defined with kprintf and bprintf). This PR will fix it.

@vasily-kirichenko
Copy link
Contributor

@dsyme It would be great if this PR were merged and the NuGet package were published. I could make the needed changes to VFPT then.

@7sharp9
Copy link
Member

7sharp9 commented Nov 20, 2014

@vasily-kirichenko Nice, I wish I had time to add more stuff to to XS. I wanted completion on the formatter semantics, so you could just choose the formatter you want when you cant remember the nemonic. Essentially the completion code would have to know you were in a sprintf etc and you had just typed %.

@dungpa
Copy link
Contributor Author

dungpa commented Nov 20, 2014

@7sharp9 You can offer a fixed set of format specifiers for completion. Or do you want a smarter mechanism?

@7sharp9
Copy link
Member

7sharp9 commented Nov 20, 2014

@dungpa Oh a fixed set would be fine, its quite a help in C# it looks like this:

screen shot 2014-11-20 at 10 26 34

I thought an F# one would be very useful.

@vasily-kirichenko
Copy link
Contributor

Yeah, it would be useful, but we don't touch intellisense in VFPT. @dungpa maybe it's better to suggest it to implement in VFT (together with intellisense colorization)?

@7sharp9
Copy link
Member

7sharp9 commented Nov 20, 2014

Main issue in XS was detecting that you were in a string argument of a string formatter function. Admittedly I never looked into it in detail.

fsgit added a commit that referenced this pull request Nov 23, 2014
Report printf specifiers from Service API
@fsgit fsgit merged commit 8c391a7 into fsharp:master Nov 23, 2014
@dungpa dungpa deleted the printf-specifiers branch November 23, 2014 22:38
tpetricek pushed a commit to tpetricek/FSharp.Compiler.Service that referenced this pull request Apr 29, 2015
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.

Report printf formatter strings or specifiers
5 participants