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

at-[s]printf to Printf stdlib #25056

Merged
merged 1 commit into from
Dec 16, 2017

Conversation

fredrikekre
Copy link
Member

Moves @printf and @sprintf to the stdlib module Printf (perhaps we can have a better name for that? FormattedPrinting? PrettyPrinting?)

Implementation is still mostly in base/printf.jl to let Base use these.

close #23929

@fredrikekre fredrikekre added deprecation This change introduces or involves a deprecation excision Removal of code from Base or the repository labels Dec 13, 2017
@fredrikekre fredrikekre added this to the 1.0 milestone Dec 13, 2017
@StefanKarpinski
Copy link
Member

Bravo and thank you!

base/sysimg.jl Outdated
@@ -362,7 +362,7 @@ import .Random: rand, rand!

# (s)printf macros
include("printf.jl")
using .Printf
import .Printf
Copy link
Member

Choose a reason for hiding this comment

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

Is this import necessary so that Printf.@printf overrides Base.Printf.@printf?

@ararslan
Copy link
Member

Looks like there's a vestigial use of Base.@sprintf in the tests somewhere

@Sacha0
Copy link
Member

Sacha0 commented Dec 16, 2017

Seems to be in shape apart from needing a rebase, a fix for the logging PR interaction, and a CI rerun?

@fredrikekre
Copy link
Member Author

Rebased, and tests passes locally

Copy link
Member

@Sacha0 Sacha0 left a comment

Choose a reason for hiding this comment

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

superficially lgtm from a skim! Thanks @fredrikekre! :)

Copy link
Member

@ararslan ararslan left a comment

Choose a reason for hiding this comment

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

👍

@StefanKarpinski
Copy link
Member

The macOS run is timing out on some kind of dependency installation. Yolo.

@StefanKarpinski StefanKarpinski merged commit 378e102 into JuliaLang:master Dec 16, 2017
@fredrikekre fredrikekre deleted the fe/stdlib-printf branch December 16, 2017 20:00
@musm
Copy link
Contributor

musm commented Dec 16, 2017

I like FormattedPrint or PrintFormatted, although if the object is just to have this module to eventually deprecate for simpler and better functionality in the future, the module name does not matter very much.

@StefanKarpinski
Copy link
Member

Yes

martinholters added a commit to martinholters/ProgressMeter.jl that referenced this pull request Dec 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation This change introduces or involves a deprecation excision Removal of code from Base or the repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

should we move printf code out of Base?
5 participants