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

rename beginswith to startswith (#9578) #9583

Merged
merged 1 commit into from
Jan 4, 2015

Conversation

bicycle1885
Copy link
Member

ref: #9578

@ivarne ivarne added the needs decision A decision on this change is needed label Jan 3, 2015
@@ -93,7 +93,7 @@ function startswith{T<:String}(stream::IO, ss::Vector{T}; kws...)
end

function startswith(stream::IO, r::Regex; eat = true, padding = false)
@assert beginswith(r.pattern, "^")
@assert Base.startswith(r.pattern, "^")
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Why do you suddenly prefix with Base.?

Copy link
Member Author

Choose a reason for hiding this comment

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

The Markdown module has already defined a function named startswith, so the name will be overridden without Base..

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Ideally the Markdown.startswith () method should be renamed now?

@one-more-minute, isn't that your code?

Copy link
Member

Choose a reason for hiding this comment

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

Yup – I'm fine with a rename if we can come up with one, but since this is self-contained (and isn't adding methods to Base.startswith) it doesn't seem like too much of a problem.

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

It's purely a stylistic issue, but might cause confusion when improving Markdown, and one might end up using the wrong function.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that's a fair point. I guess I could change it to beginswith ;) Either way I'll add it to my list of tasks when I'm updating Markdown.jl

@JeffBezanson
Copy link
Sponsor Member

The new name is fine with me. Thanks for doing all the renaming yourself. Let's see what people think.

@jakebolewski
Copy link
Member

+1 for renaming to startswith, I feel like I search for the name of this function in the manual often.

@timholy
Copy link
Sponsor Member

timholy commented Jan 4, 2015

I like startswith better, too.

@johnmyleswhite
Copy link
Member

+1

@JeffBezanson JeffBezanson merged commit 81cbec8 into JuliaLang:master Jan 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs decision A decision on this change is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants