[VSC] Accounts and contacts (1/5) #955
Conversation
mschaefer88
force-pushed
the
pr/vscode/accounts-and-contacts
branch
from
9e2b21a to
af792c2
Compare
|
@mschaefer88 Please let us know as soon as the PRs are ready to review (It seems like the PRs 1-3 have the same content). |
vscode/.vscode/extensions.json
Outdated
| @@ -0,0 +1,7 @@ | |||
| { | |||
| // See http://go.microsoft.com/fwlink/?LinkId=827846 | |||
There was a problem hiding this comment.
The JSON specification does not allow comments //.
Strict JSON parsers will reject the file. Does the VSCode parser accept the file?
I know that some JSON files contain an extra entry e.g. "_comment" or "comment", but I am not sure whether this approach can be considered as best practice (because an application could use a strict content validation and fails because of the unknown comment entry).
I suggest moving the comment to the top of the file (as in your other JSON files). I think this is still against the specification but consistent with launch.json.
mschaefer88
force-pushed
the
pr/vscode/accounts-and-contacts
branch
3 times, most recently
from
ea603e0 to
04f1ee7
Compare
mschaefer88
force-pushed
the
pr/vscode/accounts-and-contacts
branch
2 times, most recently
from
62f86c2 to
08412b3
Compare
mschaefer88
changed the title
The Saros implementation will be based on the Language Server Protocol (see https://microsoft.github.io/language-server-protocol) for data exchange. In order to enable VS Code to understand that protocol the VS Code language client has been added. Furthermore starting of the Saros LSP Server has been encapsulated.
Unintentionally removed parts have been readded (lsp) and code has been improved regarding guidelines. Overall quality has been improved. Vscodes extension build, run and publish workflow has been shifted from npm based to gradle base to embed it better into the gradle environment and to enable easier CI builds.
In order to keep the size of the extension rather small the building method has been changed to webpack. This also brings the benefit of excluding files that aren't really needed.
Introduce eslint and modify code accordingly. Furthermore gradle has been modified in order to seperate config and execute statements.
mschaefer88
force-pushed
the
pr/vscode/accounts-and-contacts
branch
2 times, most recently
from
faa1736 to
bcab531
Compare
|
Build lsp project: Check vsc with lint: Run for the first time: Documentation will follow in another PR. |
This commit will add the functionality of managing accounts and contacts of Saros. There are wizards for handling those data and necessary UI elements such as the contact and active account view.
This commit will add the functionality of managing accounts and contacts of Saros. Basic functionality like logging and config exchange is also implemented. All classes that will contain logic of Saros from a later stage will be mostly empty.
mschaefer88
force-pushed
the
pr/vscode/accounts-and-contacts
branch
from
bcab531 to
eeb7b1b
Compare
|
Issues
======
- Added 1
Complexity increasing per file
==============================
- lsp/src/saros/lsp/filesystem/LspWorkspace.java 1
- lsp/src/saros/lsp/extensions/server/SarosResultResponse.java 1
- lsp/src/saros/lsp/extensions/server/SarosResponse.java 1
- lsp/src/saros/lsp/filesystem/LspPath.java 4
- lsp/src/saros/lsp/context/ProxyContextFactory.java 1
- lsp/src/saros/lsp/net/session/NegotiationHandler.java 1
- lsp/src/saros/lsp/ui/UISynchronizerImpl.java 1
- lsp/src/saros/lsp/extensions/client/dto/ShowMessageParams.java 2
- lsp/src/saros/lsp/editor/EditorManager.java 1
- lsp/src/saros/lsp/editor/Editor.java 1
- lsp/src/saros/lsp/filesystem/LspReferencePoint.java 1
- lsp/src/saros/lsp/filesystem/LspFile.java 3
- lsp/src/saros/lsp/extensions/client/dto/WorkDoneProgressEnd.java 1
- lsp/src/saros/lsp/filesystem/LspResource.java 3
- lsp/src/saros/lsp/activity/InconsistencyHandler.java 1
- lsp/src/saros/lsp/ui/UIInteractionManager.java 2
- lsp/src/saros/lsp/preferences/LspPreferenceStore.java 5
- lsp/src/saros/lsp/utils/FileUtils.java 6
- lsp/src/saros/lsp/filesystem/LspContainer.java 5
- lsp/src/saros/lsp/extensions/client/dto/WorkDoneProgressBegin.java 1
- lsp/src/saros/lsp/filesystem/LspFolder.java 4
- lsp/src/saros/lsp/filesystem/WorkspacePath.java 2
- lsp/src/saros/lsp/extensions/client/dto/WorkDoneProgressReport.java 1
- lsp/src/saros/lsp/activity/FileActivityHandler.java 1
- lsp/src/saros/lsp/extensions/server/connection/ConnectionService.java 2
- vscode/src/types/eventAggregator.ts 2
- lsp/src/saros/lsp/SarosLifecycle.java 1
- lsp/src/saros/lsp/net/SubscriptionAuthorizer.java 3
- lsp/src/saros/lsp/monitoring/remote/LspRemoteProgressIndicator.java 1
- lsp/src/saros/lsp/context/SessionContextFactory.java 1
- lsp/src/saros/lsp/extensions/server/contact/dto/ContactDto.java 1
- lsp/src/saros/lsp/context/LspContextFactory.java 1
- lsp/src/saros/lsp/monitoring/ProgressMonitor.java 5
- lsp/src/saros/lsp/context/UIContextFactory.java 1
- lsp/src/saros/lsp/context/CoreContextFactory.java 1
- lsp/src/saros/lsp/extensions/client/dto/WorkDoneProgressCreateParams.java 1
- lsp/src/saros/lsp/extensions/server/contact/ContactService.java 3
- lsp/src/saros/lsp/extensions/client/dto/ProgressParams.java 1
- lsp/src/saros/lsp/extensions/server/document/DocumentServiceImpl.java 1
- lsp/src/saros/lsp/extensions/server/workspace/WorkspaceServiceImpl.java 1
- lsp/src/saros/lsp/preferences/LspPreferences.java 1
- lsp/src/saros/lsp/monitoring/remote/LspRemoteProgressIndicatorFactory.java 1
- lsp/src/saros/lsp/filesystem/PathFactory.java 3
- vscode/src/utils/timeout.ts 1
- lsp/src/saros/lsp/extensions/server/account/AccountService.java 1
- lsp/src/saros/lsp/context/FileSystemContextFactory.java 1
- vscode/src/utils/regex.ts 1
Complexity decreasing per file
==============================
+ lsp/src/saros/lsp/SarosLauncher.java -3
Clones added
============
- lsp/src/saros/lsp/filesystem/LspPath.java 2
- lsp/src/saros/lsp/filesystem/LspFile.java 1
- lsp/src/saros/lsp/preferences/LspPreferenceStore.java 1
- lsp/src/saros/lsp/filesystem/LspContainer.java 1
See the complete overview on Codacy |
|
|
||
| @Override | ||
| public WorkspaceService getWorkspaceService() { | ||
| return this.workspaceService; | ||
| } | ||
|
|
||
| @Override | ||
| public IAccountService getSarosAccountService() { |
| } | ||
|
|
||
| @Override | ||
| public IContactService getSarosContactService() { |
| } | ||
|
|
||
| @Override | ||
| public IConnectionService getSarosConnectionService() { |
| return CompletableFuture.completedFuture(null); | ||
| } | ||
|
|
||
| @Override | ||
| public void exit() { | ||
| log.info("exit"); | ||
| this.exitListeners.forEach(listener -> listener.run()); |
There was a problem hiding this comment.
callback or hook is the correct naming instead of listener, especially when you call run
| this.documentService = documentService; | ||
| this.connectionService = connectionService; | ||
| this.workspaceService = workspaceService; | ||
| } | ||
|
|
||
| @Override | ||
| public CompletableFuture<InitializeResult> initialize(InitializeParams params) { |
There was a problem hiding this comment.
Why do you return a Future when it is always completed when this method return ?
| public Editor(IFile file) throws IOException { | ||
| super.setUri("file:///" + file.getReferencePointRelativePath().toString()); | ||
|
|
||
| try (InputStream stream = file.getContents()) { |
There was a problem hiding this comment.
You should not do "heavy work" in a CTOR. Loading contents from a medium is such a case.
| * A generic response for requests that indicate either success or failure and have no return value | ||
| * itself. | ||
| */ | ||
| public class SarosResponse { |
There was a problem hiding this comment.
What is the point of "wrapping" exceptions like that ?
| } | ||
|
|
||
| @Override() | ||
| public CompletableFuture<SarosResultResponse<AccountDto[]>> getAll() { |
There was a problem hiding this comment.
Are there ANY ?! plans to make these methods async in the future ? If not then please drop this CompleteableFuture stuff.
|
|
||
| CompletableFuture<SarosResponse> c = new CompletableFuture<SarosResponse>(); | ||
|
|
||
| Executors.newCachedThreadPool() |
There was a problem hiding this comment.
Have you read the API what this method does ?! Do you think it is that smart to spawn a thread pool for each method invocation of add ?
| public void setContents(InputStream input) throws IOException { | ||
|
|
||
| /* | ||
| * We write the new contents to a temporary file first, then move that |
There was a problem hiding this comment.
I really hate such implementations, especially if the TMP directory is on a different drive.
| import saros.synchronize.UISynchronizer; | ||
|
|
||
| /** Implementation of {@link UISynchronizer}. */ | ||
| public class UISynchronizerImpl implements UISynchronizer { |
There was a problem hiding this comment.
If you do not correctly implement that class there is no reason to continue further development. I do not know how to access the VSC IDE UI/Main thread through JAVA. However the Jupiter Algorithm on the client side will not work correctly if the transformation is done in another thread than the thread that is responsible for changing editor contents on the client side.
If you dig down through the Github Repo you will find such a fix for Eclipse, changing the thread access (I guess it is about 10 years old).
There was a problem hiding this comment.
This pull request is the first of five to completely integrate the current state of "Saros for VS Code" into Saros.
This pull request contains all implementations regarding account and contact management.
It will integrate the needed base functionalities like connecting to Saros instance, UI elements
and other basics to get the extension and Saros running.