-
Notifications
You must be signed in to change notification settings - Fork 456
Update comment maps #680
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
Update comment maps #680
Conversation
6ad6401 to
2c8bbda
Compare
sandersn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Newline formatting.
| "xmlhttprequest-upload": "Returns the associated XMLHttpRequestUpload object. It can be used to gather transmission information when data is transferred to a server.", | ||
| "xmlhttprequest-send": "Initiates the request. The body argument provides the request body, if any, and is ignored if the request method is GET or HEAD.\nThrows an \"InvalidStateError\" DOMException if either state is not opened or the send() flag is set.", | ||
| "xmlhttprequest-abort": "Cancels any network activity.", | ||
| "xmlhttprequest-overridemimetype": "Acts as if the `Content-Type` header value for response is mime.\n(It does not actually change the header though.)\nThrows an \"InvalidStateError\" DOMException if state is loading or done.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The newlines make reading documentation a lot easier for long comments. I tested VS, VS Code and emacs.
VS and emacs treat newline as newline.
VS Code treats two-spaces-before-newline as newline -- that is, it's interpreting the comment as markdown.
I think the right thing to do is to convert the original newlines into ' \n' so that both approaches will look good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did the innerText way because the original newlines were for spec authors who edit raw HTML, which tend to look arbitrary when extracted as comments. In the worst case a full line only included a single word, which was perfectly reasonable in raw HTML because it was wrapped by an anchor element.
That said, I fixed a bug that <p> didn't insert double newlines, which should fix VS Code comment readability. If it still doesn't look good, then I can skip and defer the newline formatting part here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. I guess it's OK, but maybe we need to have our own wrapping algorithm eventually? The editors I tried don't wrap on their own [1], which becomes pretty hard to read.
[1] VS wraps to the width of the screen, emacs wraps to the width of the window. These are often nearly the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but maybe we need to have our own wrapping algorithm eventually?
Hmm, I'll try it.
Applied several fixes: