lf-lang / vscode-lingua-franca

Lingua Franca extension for Visual Studio Code
Other
5 stars 3 forks source link

Dependency checks and environment setup assistance #55

Closed petervdonovan closed 2 years ago

petervdonovan commented 2 years ago

This branch adds checks for various dependencies, with corresponding messages and suggested install commands in the UI. The new behavior is described mostly declaratively in check_dependencies.ts.

This feature has been exceedingly difficult to test. Tests exist, but coverage is incomplete. This is not good, but I think it is acceptable because when the extension sends text to the VS Code terminal, it does not append a terminating newline. This means the user can review the suggested install/update command before pressing Enter if CI is supplemented with manual tests.

petervdonovan commented 2 years ago

There are some tricky UI questions to be resolved here.

Currently, the user is presented with a notification that will typically have a button labeled either "View download page", "Update", or "Install". If they click a button that says "Update" or "Install", an update/install command is sent to the VS Code integrated terminal, which pops up on the user's screen. The problem is that the update/install command might not be what the user wants -- for example, maybe the user wants to manage their packages using brew, but this command bypasses that to do the installation using a bash script. Therefore, I decided to make the user press the Enter key to execute the command in the integrated terminal. Alternatives include:

Another issue is what happens when the user performs an action that multiple dependencies depend on. It could be problematic for two messages to pop up at once, because the user may take several seconds to handle the first message, during which time the second message may disappear. (It takes 20 seconds for a message to disappear.)

lhstrh commented 2 years ago

Currently, the user is presented with a notification that will typically have a button labeled either "View download page", "Update", or "Install". If they click a button that says "Update" or "Install", an update/install command is sent to the VS Code integrated terminal, which pops up on the user's screen. The problem is that the update/install command might not be what the user wants -- for example, maybe the user wants to manage their packages using brew, but this command bypasses that to do the installation using a bash script. Therefore, I decided to make the user press the Enter key to execute the command in the integrated terminal. Alternatives include:

* Providing more detail in the dialog/button so that the user knows in advance what command it will execute, then executing it in the terminal without waiting for the user to press Enter

* Showing a "[quick pick](https://github.com/microsoft/vscode-extension-samples/tree/main/quickinput-sample)" (sort of like a drop-down menu) that lists all the possible install commands, and lets the user choose one of them. This would have to happen in addition to showing a message explaining why the dependency is needed, because quick picks do not come with explanatory messages.

I think that either of these alternatives are better than requiring the user to press Enter. I would go for the option that is easiest to implement. The key is that a user must be 1) given sufficient information to do a manual install and 2) given the opportunity to back out of an automatic install.

Another issue is what happens when the user performs an action that multiple dependencies depend on. It could be problematic for two messages to pop up at once, because the user may take several seconds to handle the first message, during which time the second message may disappear. (It takes 20 seconds for a message to disappear.)

I'm not sure what to do about this one. Perhaps we shouldn't worry about it considering that a "retry" can always be performed to get to messages that disappeared?

petervdonovan commented 2 years ago

I still need to check in the GUI that this behaves as intended, but the tests pass, and I don't anticipate major changes in this PR before it gets merged.

petervdonovan commented 2 years ago

Another caveat: This PR introduces nondeterministic test failures like this:

Downloading VS Code 1.68.1 from https://update.code.visualstudio.com/1.68.1/darwin/stable
node:events:505
      throw er; // Unhandled 'error' event
      ^
Error: read ECONNRESET
    at TLSWrap.onStreamRead (node:internal/stream_base_commons:217:20)
Emitted 'error' event on ClientRequest instance at:
    at TLSSocket.socketErrorListener (node:_http_client:454:9)
    at TLSSocket.emit (node:events:527:28)
    at emitErrorNT (node:internal/streams/destroy:157:8)
    at emitErrorCloseNT (node:internal/streams/destroy:122:3)
    at processTicksAndRejections (node:internal/process/task_queues:83:21) {
  errno: -54,
  code: 'ECONNRESET',
  syscall: 'read'
}

These only occur on the MacOS runners, but they occur pretty often -- something like half the time. I think that these failures are bad, but acceptable.

lhstrh commented 2 years ago

Would caching be a good strategy to mitigate the connection issues?

petervdonovan commented 2 years ago

After the latest bugfix, I just got this branch to pass a basic smoke test (by installing Rust from within the VS Code editor). I think that what we have now should be better than nothing.