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

Fixes issue #101 #104

Closed
wants to merge 4 commits into from
Closed

Fixes issue #101 #104

wants to merge 4 commits into from

Conversation

melo0187
Copy link
Contributor

@melo0187 melo0187 commented Apr 7, 2014

Fixes issue #101: Sharing videos from youtube app to xbmcremote stopped working because
Intent.EXTRA_TEXT contained more than just the url and the url itself
changed to a short version (youtu.be/videoID)

Carmelo Scollo added 2 commits April 7, 2014 18:46
Sharing videos from youtube app to xbmcremote stopped working because
Intent.EXTRA_TEXT contained more than just the url and the url itself
changed to a short version (youtu.be/videoID)
@melo0187
Copy link
Contributor Author

melo0187 commented Apr 7, 2014

I'm sorry that this pull request also contains a "revert" commit I did by accident. I didn't mean to add this useless commit to the pull request nor did I mean to revert the commit in the first place. This is my first fork->-branch->fix->request-pull, hence the mistake. I would be glad to correct this mistake, if anyone could be so kind to explain how.

Thanks in advance!
melo

I accidentally reverted commit 6380b3d. This commit is to undo this accidental revert.
@gthazmatt
Copy link

I don't know the policy of this group, but it's common to want to keep the history clean, so they may reject the pull request and ask that you fix your branch before submitting another. You would need to use rebasing to fix it (http://git-scm.com/book/en/Git-Branching-Rebasing). In this position (I see you just added an undo commit), I would do "git rebase -i HEAD~4" (can also use the hash of the common branch) and simply remove both the initial and undo commits from the commit list. The changes would be discarded, and you would be left with just the one commit on top of master. You would also need to push to a different remote repository.

@tombriden
Copy link
Collaborator

Normally you'd be right but as this is a small, rarely updated project I'll just cherry pick the relevant commit and close off this request when I'm at my pc tomorrow :)

@melo0187
Copy link
Contributor Author

melo0187 commented Apr 7, 2014

@gthazmatt thank you very much for the reply. Even if not needed in this situation as @tombriden said, I will look into rebasing so that I can learn how to fix this silly mistakes, because all I wanted was to contribute a fix and not litter the commit history. I'm sorry for the inconvenience. As I said this is basically my first git/github experience and I'm still learning.

Found a better regex working regardless of the existence of protocol or "www" in the url and being more robust and readable in general. This regex also returns only one capturing group containing only the thing we are looking for, the video ID.
Tests: http://www.regexr.com/38m12
@tombriden
Copy link
Collaborator

I've cherry-picked the 2 commits so will now close this off. If you need to do any further changes then start a new branch and pull request.

@tombriden tombriden closed this Apr 8, 2014
@melo0187 melo0187 deleted the fix-play-on-xbmc branch April 16, 2014 11:12
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.

3 participants