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.

Sign up for GitHub

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

Jump to bottom

add sizehint keyword argument to IOBuffer #25944

Merged
merged 8 commits into from
Feb 9, 2018
Merged

add sizehint keyword argument to IOBuffer #25944

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
767ea34
add sizehint keyword argument to IOBuffer
bicycle1885 Feb 8, 2018
19d9b34
use sizehint in Base
bicycle1885 Feb 8, 2018
f24f51d
fix NEWS.md
bicycle1885 Feb 8, 2018
2e3f5e8
fill PR serial [ci skip]
bicycle1885 Feb 8, 2018
231146d
remove redundant arguments and operations
bicycle1885 Feb 9, 2018
65f45fb
fixup! remove redundant arguments and operations
bicycle1885 Feb 9, 2018
4ee880b
check the length of arguments for safety
bicycle1885 Feb 9, 2018
0103483
make sizehint higer precedence than maxsize
bicycle1885 Feb 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions base/strings/io.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ tostr_sizehint(x::Float64) = 20
tostr_sizehint(x::Float32) = 12

function print_to_string(xs...; env=nothing)
if isempty(xs)
return ""
end
Copy link
Member

Choose a reason for hiding this comment

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

isempty(xs) && return "" would be more idiomatic (and shorter).

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it is a personal taste rather than an idiom. I'd like to use if in this kind of case because it is easier to read control flow.

Copy link
Member

Choose a reason for hiding this comment

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

The && syntax is widely used in Base to check for small corner cases at the beginning of functions. Hence I would say it is idiomatic in Base. (The word “idiomatic” refers to choices that are questions of taste/usage rather than grammar.)

Obviously the two are technically equivalent, so I won’t insist.

Copy link
Member

Choose a reason for hiding this comment

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

(The question in cases like this is not your personal taste, but rather the dominant style of the project being patched.)

Copy link
Member Author

Choose a reason for hiding this comment

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

But I can find lots of if <condition> <newline> <single statement> <newline> end usage in Base (I don't know stats on which is more dominant) and cannot find a guideline on the style, so I used the words "personal taste" here. Anyway, I think this is off-topic.

Copy link
Member

Choose a reason for hiding this comment

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

Both styles are definitely acceptable.

# specialized for performance reasons
s = IOBuffer(sizehint=tostr_sizehint(xs[1]))
if env !== nothing
Expand Down