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

Title with colon #1

Open
hemanth opened this issue Jan 6, 2015 · 7 comments
Open

Title with colon #1

hemanth opened this issue Jan 6, 2015 · 7 comments

Comments

@hemanth
Copy link

hemanth commented Jan 6, 2015

curl 'https://www.youtube.com/watch?v=ci0Pihl2zXY' -s | article-title

Results in:

#Edit2014

Commenting out this RE in the code, gives the right result that being Wikipedia: #Edit2014

//cc @sindresorhus

@hemanth
Copy link
Author

hemanth commented Jan 10, 2015

@sindresorhus Meow :) ?

@sindresorhus
Copy link
Owner

Don't have time to look into this right now, but looks like I need to give higher priority to the last part of the title.

PR welcome in form of a fix or even just a failing test-case.

@hemanth
Copy link
Author

hemanth commented Jan 10, 2015

👍

@icyflame
Copy link
Contributor

@hemanth

I have done some work on this.

I have basically added two test cases.

One of them uses the node-curl package, and curls the page from Youtube and then finds the title. This returns an appropriate response. (Wikipedia: Edit14)

The second test is using a fixture. I have stored the HTML in a new fixture file, and then run the same test using a fixture. Surprisingly, this returns the result #Edit14 )

I am not able to figure out why the two differing results are found.

Find my new tests at: https://github.com/icyflame/article-title/blob/add-tests-for-titles-with-colon/test.js#L23

Output of the new test:

image

The one test that passes is the one which curls the youtube HTML.

cc: @sindresorhus

icyflame added a commit to icyflame/article-title that referenced this issue Jun 16, 2015
kevva pushed a commit that referenced this issue Jun 16, 2015
kevva pushed a commit that referenced this issue Jun 16, 2015
@sindresorhus sindresorhus changed the title Title with colon. Title with colon Mar 25, 2017
@astoilkov
Copy link

I am seeing a lot of incorrect results when using article-title on YouTube. Example:

  • curl https://www.youtube.com/watch\?v\=aJOTlE1K90k | npx article-title-cli gives Maroon 5 instead of Maroon 5 - Girls Like You ft. Cardi B

I think two things can be done the docTitle stripping could be improved. Second, a new logic could be added where if you have only one h1 tag on the page this tag is considered the title.

@astoilkov
Copy link

@sindresorhus If you agree with the new logic regarding only one h1 on the page I can implement it and send a pull request.

@sindresorhus
Copy link
Owner

Yup. I agree. PR welcome :)

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

No branches or pull requests

4 participants