-
Notifications
You must be signed in to change notification settings - Fork 185
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
Support document formatting #8 #10
Conversation
public function formatting(TextDocumentIdentifier $textDocument, FormattingOptions $options) | ||
{ | ||
$nodes = $this->asts[$textDocument->uri]; | ||
if (empty($nodes)){ |
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.
Space missing before {
This needs a unit test |
} | ||
$prettyPrinter = new PrettyPrinter(); | ||
$edit = new TextEdit(); | ||
$edit->range = new Range(new Position(0, 0), new Position(PHP_INT_MAX, PHP_INT_MAX)); |
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.
@dbaeumer is it okay to do this to update the whole document?
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.
@felixfbecker yes and no. It is always best if an extension provides a minimal edit set since it will preserve the most markers attached to the text. We have code in place that runs a diff in the whole document gets replaced to compute some more minimal diff. However this might be suboptimal as well. So the more fine grain your edits are the better.
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.
@dbaeumer Of course a minimal text edit is better, but the default implementation of PHPParser's PrettyPrinter will just spit out the new whole new content. We may customize the behavior later, but for now we just wanted to get the feature done. My question was only about wether it is okay to use a very big number (PHP_INT_MAX) to make it update the whole document (even though the EOF offset is much smaller) or if that is not protocol-conform and causes problems.
Looking forward to using this while developing on the language server itself! |
LGTM |
Closes #8