-
Notifications
You must be signed in to change notification settings - Fork 92
Use JuliaFormatter instead of DocumentFormat #972
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
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
0d9c519
Use JuliaFormatter instead of DocumentFormat
non-Jedi 20d640e
Merge branch 'juliaformatter' of https://github.com/non-Jedi/Language…
pfitzseb ae37ebf
update
pfitzseb b037a74
respect .JuliaFormatter.toml in workspace
pfitzseb ba8b7cd
fix logic
pfitzseb b773e31
selection formatting
pfitzseb e07dc69
require JuliaFormatter 0.15.4
pfitzseb 550f27f
Merge branch 'master' into sp/juliaformatter
davidanthoff e82a6a0
Merge branch 'master' into sp/juliaformatter
pfitzseb 9bd2f85
update
pfitzseb be5f3bc
fixes
pfitzseb File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Does this request get hit every key stroke? If so it's not ideal to search the disk each time. I can't remember whether we're able to have a file watch from within the languageserver which may be why this has been 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.
We could just add those files to https://github.com/julia-vscode/julia-vscode/blob/master/src/extension.ts#L232, and then we would get normal LSP messages about their content etc. That seems in general 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.
No, only when explicitly requested.
IIUC we'd only be notified by FS events or when the user opens/changes the
.JuliaFormatter.toml. The client won't just dump all matching files into the server.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.
Not sure what you mean by that. We could just handle
.JuliaFormatter.tomlfiles the same way we handle all.jlfiles in the workspace, right? Then they would be in-memory always?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.
Is that so? I thought we'd load most files directly from disk?
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.
Yes, we load initially, but then we don't need to reload because we get notifications whenever they are updated. If this is not hit with every keystroke that probably doesn't matter that much, though.
Having said that, the LSP will have to move to a model at some point where language servers just don't touch the file system at all and instead can get all content from the client, just to make things work properly with virtual file systems that are supported in the client. At the moment there is not support for that in the protocol itself, but we would probably be better prepared for that if we made these files "normal" files. On the other hand, lets not make that a roadblock for this PR :) The
MinimalStyleis, though...