Repository navigation
[wip] check validity of branch name - #759
franciscod wants to merge 10 commits into
Conversation
with support for git2::Branch::name_is_valid, via git_branch_name_is_valid
|
that |
|
We need a git2 release. We can’t depend on a revision |
(all this is checked later anyway)
franciscod
left a comment
There was a problem hiding this comment.
i don't know how I started this review and I don't know how to cancel it
what do you mean? |
I accidentally started some kind of review on the github UI and couldn't cancel it, only add a comment |
extrawurst
left a comment
There was a problem hiding this comment.
aside this small improvement we mostly need to wait for a git2-rs release. you can ask Alex in your PR to cut a release
| fn validate_input(&mut self) -> Result<()> { | ||
| let branch_name = self.input.get_text().as_str(); | ||
|
|
||
| self.already_exists = false; |
There was a problem hiding this comment.
since we only show either invalid_name or already exists (which is ok) we also only need to store the validity in an enum. this also means we do not need to test the for the other if the first check already fails.
There was a problem hiding this comment.
ah, makes sense! will do. thanks!
| } | ||
|
|
||
| if let Event::Key(e) = ev { | ||
| self.validate_input()?; |
There was a problem hiding this comment.
does this even work? IMO every key consumed by self.input will not even go here
| self.already_exists = true; | ||
| } | ||
| } | ||
| self.invalid_name = sync::branch_name_is_valid(branch_name)?; |
There was a problem hiding this comment.
this cannot work, this is inverted
| let branch_name = self.input.get_text().as_str(); | ||
|
|
||
| self.already_exists = false; | ||
| self.invalid_name = false; |
There was a problem hiding this comment.
this assignment is useless
|
sorry for the rushed commit. I was overconfident that it would work. I'll take my time and test every corner of it before requesting a review again. |
|
@franciscod I will mark this as draft in the meantime, please ping me when you want me to check it out again 👍 |
|
@franciscod do you still plan to finish this? |

(Okay, here we go, rust PR number two 💃)
I'm definitely certain that this is ugly code. It works, it's probably correct, but it's not pretty. I don't know what I should do to improve it.
Feedback would be greatly appreciated :)