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

CLI improvements. #60

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

CLI improvements. #60

wants to merge 5 commits into from
+18 −6

Conversation

aroneiermann
Copy link
Contributor

Better error handling.
Printing article.content not article.html. ;)

@aroneiermann
Copy link
Contributor Author

@luin I'm not sure if cli.js is the right place to add the h1 stuff (87c76bc). Perhaps it would be better to detect that article.content contains no h1 element and then add one?

@aroneiermann
Initializing variable.
5ac1eff 
Removes starting "undefined" from html.
@luin
Copy link
Owner

luin commented Sep 17, 2015

What if including article.title in the <title />? So the html would be '<html><head><title>' + article.title + '</title></head><body>' + article.content + '</body>';

@aroneiermann
Copy link
Contributor Author

I think that article.content should contain a header, specially a h1 tag. ( @luin What do you think about this point?).
But in the most websites I have parsed, article.content doesn't contain a header.
Title and heading are not the same, but in most cases the title and the heading are relative similar. So I thought about to add the article.title as a header (h1) to article.content.
Perhaps it could also contain a title tag (Not sure about it).

@luin
Copy link
Owner

luin commented Sep 19, 2015

Makes sense. We can add a h1 tag with the content of article.title when the article.content doesn't contain a h1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aroneiermann @luin