Skip to content

Soft wrapping for comments #2102

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

Merged
merged 6 commits into from
Nov 2, 2017
Merged

Conversation

topecongiro
Copy link
Contributor

@topecongiro topecongiro commented Oct 31, 2017

This PR is a first step to implementing soft wrapping for comments (cc #535).

Note that this PR does NOT implement soft wrapping for doc comments (/// or //!). Currently each line of doc comments are rewritten on its own due to how it's implemented in AST (a vector of ast::Attribute). Fixing this will be in a different PR, hopefully.

@topecongiro
Copy link
Contributor Author

Also this PR prevents URLs from being split, in a heuristic way.

Copy link
Member

@nrc nrc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, r+ with the extension to has_url

src/comment.rs Outdated
/// Returns true if the given string MAY include URLs or alike.
fn has_url(s: &str) -> bool {
// This function may return false positive, but should get its job done in most cases.
s.contains("https://") || s.contains("http://")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add "file" and "ftp" schemes too please?

@topecongiro
Copy link
Contributor Author

Thank you for the review! Updated.

@nrc nrc merged commit 809e06e into rust-lang:master Nov 2, 2017
@nrc
Copy link
Member

nrc commented Nov 2, 2017

Thanks!

@topecongiro topecongiro deleted the soft-wrapping-comments branch November 2, 2017 02:26
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.

2 participants