intersystems-community / vscode-objectscript

InterSystems ObjectScript extension for Visual Studio Code
https://docs.intersystems.com/components/csp/docbook/DocBook.UI.Page.cls?KEY=GVSCO
Other
108 stars 49 forks source link

Add command for opening InterSystems documents #1398

Closed isc-bsaviano closed 3 months ago

isc-bsaviano commented 3 months ago

This PR fixes #1379. The new Open InterSystems Document... command can be opened from the command palette and it provides a UI for browsing all files in the connected server-namespace. The user can either pick an item in the list, or type text into the filter box and press "Enter" to submit that. The UI should function very similarity to the built-in simple file dialog. I would like to assign a keybinding to this command, but I couldn't find a good one so I'm open to suggestions.

output

isc-bsaviano commented 3 months ago

@isc-rsingh @gjsjohnmurray How do you feel about using a chord ask the keybinding for this? Ctrl+K Alt+O (Cmd+K Alt+O on Mac) is available. We could also overload Ctrl+K Shift+O (Cmd+K Shift+O on Mac), which is only use din the diff editor. This is the best that I could come up with ending with O.

isc-rsingh commented 3 months ago

How about Ctrl+K Ctrl+O (Mac: Cmd+K Cmd+O) which seems easy to type and available.

isc-bsaviano commented 3 months ago

That's taken on Windows

isc-rsingh commented 3 months ago

Ugh. Now I see the semantics are different on Windows and Mac. On Mac, Cmd+O does both the open file and open folder semantics. While on Windows there are 2 separate keybindings, Ctrl+O for open file and the chord Ctrl+K Ctrl+O for open folder. I think the best available option is the chord Ctrl+K Ctrl+Shift+O and Mac Cmd+K Cmd+Shift+O?

isc-bsaviano commented 3 months ago

I think Ctrl+K Alt+O is better because it's fewer keys but I'm not sure any of these are natural/easy to use.

isc-rsingh commented 3 months ago

My suggestion was based on the convenience of not lifting the finger off of Ctrl (or Cmd) when typing the 2nd part of the chord, which is why I preferred it over a chord that changes all the finger positions, but its not a huge difference. You could go with either and see if we get feedback.

isc-bsaviano commented 3 months ago

That's a good point. Let's see what John thinks.

gjsjohnmurray commented 3 months ago

Adding keybindings to new versions of existing extensions brings the risk that some current users will experience problems because the combo we choose clashes with another extension or a keybinding they set up.

I'd be inclined not to set one.

isc-bsaviano commented 3 months ago

I'm fine with not setting one

isc-bsaviano commented 3 months ago

If you're both happy with this I'll merge it

gjsjohnmurray commented 3 months ago

I shan't have time to review / try this before tomorrow, so go ahead if you want.

isc-bsaviano commented 3 months ago

No problem John, I'll wait for you review

gjsjohnmurray commented 3 months ago

How about adding the command to the Explorer view title menu, perhaps with go-to-file as its icon?

gjsjohnmurray commented 3 months ago

Mapped .inc documents are not being hidden. To reproduce this, be connected to a namespace such as USER, set toggles to show system documents and to hide mapped ones. Observe that some %-packages still get listed. Drill into one and discover that you are being offered .inc documents despite them being mapped here from another database.

gjsjohnmurray commented 3 months ago

It would be nice if the three toggle buttons remembered their state between invocations of the selector, at least for the duration of my session, and perhaps persist per workspace.

It would also be nice if I could see the state of each. AFAIK the only way of doing this at the moment might be to create 6 of our own icons, 3 to represent "on" and 3 "off", then manipulate the buttons array. Perhaps the "on" icons would have a solid outline.

Alternatively I could have a go at a PR for https://github.com/microsoft/vscode/issues/185356, or better still my suggestion at https://github.com/microsoft/vscode/issues/221397#issuecomment-2225357158 gets accepted.

isc-bsaviano commented 3 months ago

How about adding the command to the Explorer view title menu, perhaps with go-to-file as its icon?

You mean the default File Explorer, right? I will see if I can add the command there.

Mapped .inc documents are not being hidden. To reproduce this, be connected to a namespace such as USER, set toggles to show system documents and to hide mapped ones. Observe that some %-packages still get listed. Drill into one and discover that you are being offered .inc documents despite them being mapped here from another database.

This sounds like a bug in the %Library.RoutineMgr_StudioOpenDialog() query. Please report it to the WRC.

It would be nice if the three toggle buttons remembered their state between invocations of the selector, at least for the duration of my session, and perhaps persist per workspace.

It would also be nice if I could see the state of each. AFAIK the only way of doing this at the moment might be to create 6 of our own icons, 3 to represent "on" and 3 "off", then manipulate the buttons array. Perhaps the "on" icons would have a solid outline.

Alternatively I could have a go at a PR for microsoft/vscode#185356, or better still my suggestion at microsoft/vscode#221397 (comment) gets accepted.

I completely agree; this is why I opened that issue a while ago. I think your suggestion or the colored buttons would both be good solutions to this problem.

gjsjohnmurray commented 3 months ago

You mean the default File Explorer, right? I will see if I can add the command there.

Yes.

Another bit of feedback:

image

Could the title be reworded "Open a document in ..."?

isc-bsaviano commented 3 months ago

@gjsjohnmurray I have that clumsy language to allow for the API caller to change the suffix but since we have no other callers now I can change that

isc-bsaviano commented 3 months ago

@gjsjohnmurray I wasn't able to add the command inline using an icon, but it does appear in the ... menu next to the other icons.

gjsjohnmurray commented 3 months ago

Did you try adding "group": "navigation" to the new entry?

isc-bsaviano commented 3 months ago

That worked! I tried "group": "inline" originally, which didn't work.

gjsjohnmurray commented 3 months ago

This sounds like a bug in the %Library.RoutineMgr_StudioOpenDialog() query. Please report it to the WRC.

Logged in WRC as 989244

gjsjohnmurray commented 3 months ago

On a multiroot isfs workspace could the "Select workspace folder" selector default to the folder of the current active document rather thann the first folder?

isc-bsaviano commented 3 months ago

The showWorkspaceFolderPick() doesn't allow you to set a default value. I suppose we could create a custom QuickPick with the default set, but that seems a little overkill to me.

gjsjohnmurray commented 3 months ago

I suppose we could create a custom QuickPick

I agree it's not worth the effort of doing that.

isc-bsaviano commented 3 months ago

Thanks for all the feedback. I think this is ready to go. If/when we get toggle buttons or colored buttons for QuickPick I will add that to all places in ur extension that would benefit, including this command.

gjsjohnmurray commented 3 months ago

Logged in WRC as 989244

Confirmed by WRC, and Jira DP-433169 opened.

gjsjohnmurray commented 3 months ago

Logged in WRC as 989244

Confirmed by WRC, and Jira DP-433169 opened.

Fixed by MAK5922 (Do not display routines from IRISLIB/IRISSYS when 'mapped=0' parameter for StudioOpenDialog query) which may appear in 2024.3