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

git-missing: do proper argument parsing. Fix #562 #565

Merged

Conversation

nicolaiskogheim
Copy link
Collaborator

@nicolaiskogheim nicolaiskogheim commented Aug 6, 2016

Ref #562

This is a rewrite of git-missing to make it behave like the docs says it will.

I have tested by running these commands, which should be enough:

$ git missing feature
$ git missing master feature
$ git missing --no-merges feature
$ git missing --no-merges master feature
$ git missing --no-merges master feature --no-merges

That last one may seem stupid, but argument parsing, and especially passing arguments along to other commands in bash can be very tricky, so that one is important. git log doesn't care about duplicate arguments, so that's alright.

Only tested on OSX, with bash 4.3.46(1)-release.
I don't think there is anything that would make this crash on other systems, but I will test it on FreeBsd and Arch tomorrow.


Possible future enhancements:

git-missing will get confused with a call looking like this: git missing --option value firstbranch secondbranch. I don't know if git log accepts any argument on the form --arg value. I scanned man git-log and it doesn't look like it, but git log understands hundreds of different arguments it looks like, so I may be wrong.

@nuragic
Copy link

nuragic commented Aug 6, 2016

Thanks so much for this one!

@qw3rtman
Copy link
Collaborator

qw3rtman commented Aug 6, 2016

Tested and working on FreeBSD 10.3 with GNU bash, version 4.3.46(1)-release (amd64-portbld-freebsd10.1) and git version 2.9.0. I also tested git missing master feature --no-merges and git missing feature --no-merges.

@nicolaiskogheim
Copy link
Collaborator Author

Thanks, @qw3rtman.


I found something that will break this, and that is the -L <start>,<end>:<file>, -L :<funcname>:<file> argument. I'll consider redoing the argument loop to tackle that later.

@nicolaiskogheim nicolaiskogheim merged commit e62a658 into tj:master Aug 6, 2016
@nicolaiskogheim nicolaiskogheim deleted the git-missing-argument-parsing branch August 6, 2016 18:34
@nuragic
Copy link

nuragic commented Aug 8, 2016

@nicolaiskogheim hi! now that this change is on master, it will be automatically available in the latest release... ?

@nicolaiskogheim
Copy link
Collaborator Author

No, sorry. We will have to do a release first. When that will be I don't know.
@hemanth When do you think we'll be able to cut a new release?

@hemanth
Copy link
Collaborator

hemanth commented Aug 11, 2016

After #534 ?

@nicolaiskogheim
Copy link
Collaborator Author

Thanks, forgot about that one.

@nuragic We are waiting to resolve #532, which is partly blocked while waiting for a reply by the author of that issue. But after that, it shouldn't be too long.

@nuragic
Copy link

nuragic commented Sep 12, 2016

Hi @nicolaiskogheim,

Sorry in advance if I sound pedantic but, looking at that PR (#532) there are no news since June..

Wondering if you guys can just do a patch release...? (v4.1.1) 😉

@nicolaiskogheim
Copy link
Collaborator Author

Yeah, you're right, it has been a long time. We do have a lot of stuff that should be released.

@spacewander @hemanth @qw3rtman

@qw3rtman
Copy link
Collaborator

@nicolaiskogheim, made a new issue for further discussion: #579.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants