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

Long links wrapping option #86

Merged
merged 8 commits into from Jul 6, 2015
Merged

Long links wrapping option #86

merged 8 commits into from Jul 6, 2015

Conversation

theSage21
Copy link
Collaborator

Possible fix for #38

  • urls which are long are not wrapped if so desired.
  • old behaviour is maintained to avoid breaking someone's code
  • --no-wrap-links has been added.
  • both inline links and reference links are supported for no wrapping.
  • docs have been updated

New option --no-wrap-links
Basic link ignore added.
Conflicts:
	docs/usage.md
	html2text/__init__.py
	html2text/cli.py
@Alir3z4
Copy link
Owner

Alir3z4 commented Jul 4, 2015

Great work, @theSage21 please update the readme.md with the latest changes regarding to options.

@theSage21
Copy link
Collaborator Author

@Alir3z4 I feel the readme is getting long. The option has been added in the relevant docs section. Should i still add?

@Alir3z4
Copy link
Owner

Alir3z4 commented Jul 4, 2015

@theSage21 I feel the same way, but we can't leave it to be out-dated.One solution would be removing the options/flags from readme me and just mention the the related doc in the readme file, what do you think ?

If you're agree you can leave it out here and remove the options/flags section in another pull-request.

@theSage21
Copy link
Collaborator Author

@Alir3z4 Lets mention in the readme that all options are available in the docs and keep one option for demonstration.

@theSage21
Copy link
Collaborator Author

@Alir3z4 I will leave it as it is here. For clarity I will create another PR which has the required changes.

@stefanor
Copy link
Contributor

stefanor commented Jul 4, 2015

Without --reference-links, the behaviour is a bit weird. Maybe it should imply --reference-links, or turn a link into a reference link, if it would be wrapped (which would probably be quite a bit more complex)

@theSage21
Copy link
Collaborator Author

The implied --reference-links is a good idea. I like it.

@theSage21
Copy link
Collaborator Author

@stefanor On second thought, what if someone wants in-line links but not wrapped? The kind you get with just --no-wrap-links. I do not want to restrict people in their use of the library. What should we do?

@stefanor
Copy link
Contributor

stefanor commented Jul 5, 2015

The thing is that it's not just the link you're not wrapping, it's the rest of the paragraph. Having the paragraph not be wrapped just because it has a link in the middle of it is weird.

@theSage21
Copy link
Collaborator Author

True. --no-wrap-links implies --reference-links. 👍

@theSage21
Copy link
Collaborator Author

@Alir3z4 I removed the CLI options from README and pointed them to the docs. You can merge now.

Alir3z4 added a commit that referenced this pull request Jul 6, 2015
Long links wrapping option

Thanks @theSage21
@Alir3z4 Alir3z4 merged commit ab3c470 into Alir3z4:master Jul 6, 2015
@Alir3z4
Copy link
Owner

Alir3z4 commented Jul 6, 2015

@theSage21 @stefanor Thanks for the great job.

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.

None yet

3 participants