tlaplus / vscode-tlaplus

TLA+ language support for Visual Studio Code
MIT License
357 stars 33 forks source link

Show command "Check model with TLC" even when the model checking panel has focus #215

Open lemmy opened 3 years ago

lemmy commented 3 years ago

The "Check model with TLC" command is not shown when the model-checking panel has focus. This makes it necessary to e.g. close the panel if a user wants to add new options (via the new prompt).

Screenshot from 2021-05-25 21-09-07

/cc @klinvill

klinvill commented 3 years ago

@lemmy I whipped up a quick additional command ("TLA+: Run last model check with new options") that should work when the model checking pane was focused. Feel free to give it a whirl and let me know if it works well for you.

Ideally we could just also enable the tlaplus.model.check.run command (which shows up as "TLA+: Check model with TLC" in the command pane) when the model checking pane is open, but I wasn't able to find a good way to do it that doesn't involve manually setting the context. I'd rather not manage additional context if we don't have to so I opted to introduce a new command.

lemmy commented 3 years ago

@klinvill Thanks; I will report back on the ergonomics.

alygin commented 3 years ago

I'd rather not manage additional context

@klinvill, why don't you like this approach? We already have a couple of contexts. To me, this looks like the right way to implement the feature.

alygin commented 3 years ago

BTW, it looks like we don't need an additional context, we can reuse tlaplus.tlc.canRunAgain to enable the command.

klinvill commented 3 years ago

Ah excellent point, I should have realized that when I used tlaplus.tlc.canRunAgain as the condition for the new command.

Unfortunately there's a second challenge with re-using tlaplus.model.check.run since the checkModel() function either expects a file URI to be passed or pulls the URI from the active editor file. If we wanted to reuse this function, we would have to turn the flow into something like:

  1. If given a file URI , read the spec from the file.
  2. If the active file is a TLA+ file, read the spec from that file.
  3. If lastCheckFiles is populated, use that spec.
  4. Error, no spec to check

This is definitely feasible, but the warning messages for no active file editor or the active file not being a .tla or .cfg file are coded into the checks with those functions. So the three options I see are:

I think the 3rd option is the cleanest, but requires introducing result types that aren't used elsewhere (or throwing and catching exceptions). In light of that, my preferred approach is still the first option: a new command. @alygin which approach do you think we should take?

alygin commented 3 years ago

I'd pursue the 3rd option with introducing a result type and minor refactoring.

I think adding a new command for this specific case doesn't look justified. A new command would add nothing to the existing one in terms of functionality, so we can just expand the context of "Check model with TLC", and shorten the user path a little bit.

lemmy commented 3 years ago
  1. seems to be most in line with how the Toolbox handles its F11 shortcut.