Skip to content

Align doc comments, support single line // comments#1800

Open
let-def wants to merge 5 commits intoreasonml:masterfrom
let-def:line-comment
Open

Align doc comments, support single line // comments#1800
let-def wants to merge 5 commits intoreasonml:masterfrom
let-def:line-comment

Conversation

@let-def
Copy link
Copy Markdown
Contributor

@let-def let-def commented Jan 31, 2018

Refactor the implementation of comments:

  • doc comments are now aligned like other comments.
  • // are accepted.

Comments are now indented inside Easy_format, this enables a uniform treatment of all comments (normal comments, single line comments, doc comments, whether they are printed as atoms or attached to list wrappers).

Hence the modifications to Easy_format.
pp_print_as is used to trick Format to put newlines: pp_force_newline forces a newline in the current box, while using pp_print_as with a size larger than the current margin puts the newline in the next box that receives content.

To merge after #1799 .

@@ -0,0 +1,3 @@
module JustString : {
include Map.S; // Comment eol include
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We will need to get the syntax highlighters to be updated. Vim should be good. But imagine if you had a brace or mismatched paren in a comment.

}

and enter_comment = parse
| "//" ([^'\010']* newline as line)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@let-def Are there other kinds of newlines we need to watch out for (something like windows)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It should be compatible with windows. However, it might break with MacOs 9 😿 .

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants