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 show methods for GitRebase and RebaseOperation #20834

Merged
merged 2 commits into from
Mar 3, 2017
Merged

Add show methods for GitRebase and RebaseOperation #20834

merged 2 commits into from
Mar 3, 2017

Conversation

kshyatt
Copy link
Contributor

@kshyatt kshyatt commented Feb 28, 2017

Now you can see what RebaseOperation is happening at any one step!

@kshyatt kshyatt added display and printing Aesthetics and correctness of printed representations of objects. libgit2 The libgit2 library or the LibGit2 stdlib module labels Feb 28, 2017
ararslan
ararslan reviewed Feb 28, 2017
View reviewed changes
base/libgit2/rebase.jl Outdated
@@ -41,6 +41,11 @@ function Base.next(rb::GitRebase)
return unsafe_load(rb_op_ptr_ptr[])
end

function Base.show(io::IO, rb::GitRebase)
println(io, "GitRebase:")
println(io, "Number of operations to apply: $(count(rb))")
Copy link
Member

Choose a reason for hiding this comment

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

Could you just do "Number ...: ", count(rb) to avoid the interpolation? Might be very marginally faster

@ararslan
Copy link
Member

This is really cool! Nice work!

@kshyatt
Copy link
Contributor Author

kshyatt commented Feb 28, 2017

@tkelman does the BoundsError look ok? This mergeable?

@tkelman
Copy link
Contributor

tkelman commented Feb 28, 2017

Is PkgDev or any other code expecting that getindex to be returning nothing, or the flags to be constants instead of an Enum?

@kshyatt
Copy link
Contributor Author

kshyatt commented Feb 28, 2017 •
edited
Loading

PkgDev doesn't include rebasing at all, as far as I can tell.

A grep on PkgDev shows no rebase/Rebase/REBASE anywhere in its code.
The relevant C call here for getindex returns NULL for out-of-bounds index, it's that silent failure nothing we dislike so much.

@kshyatt
Copy link
Contributor Author

kshyatt commented Feb 28, 2017

A search for the name of the constants/enum shows nothing in Julia on GH, I can't see into people's private repos, of course.

ararslan
ararslan reviewed Mar 1, 2017
View reviewed changes
base/libgit2/rebase.jl Outdated
function Base.show(io::IO, rb::GitRebase)
println(io, "GitRebase:")
println(io, "Number: ", count(rb))
println(io, "Currently performing operation: $(current(rb)+1)")
Copy link
Member

Choose a reason for hiding this comment

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

Should remove the interpolation here too for consistency since you did so above

@kshyatt
Copy link
Contributor Author

kshyatt commented Mar 3, 2017

Good to go if CI passes?

@kshyatt
Copy link
Contributor Author

kshyatt commented Mar 3, 2017

It's the spawn failure again.

@kshyatt kshyatt merged commit 589b399 into master Mar 3, 2017
@kshyatt kshyatt deleted the ksh/showrebase branch March 3, 2017 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ararslan ararslan ararslan approved these changes

Assignees
No one assigned
Labels
display and printing Aesthetics and correctness of printed representations of objects. libgit2 The libgit2 library or the LibGit2 stdlib module
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kshyatt @ararslan @tkelman