Add openRecent query parameter #272
Conversation
If you add openRecent to the VS Code link, it will open the most recent instead of whatever is in the folder param (if there is a most recent). Also decided to open the most recent when opening from the sidebar, seemed reasonable to me anyway.
code-asher
force-pushed
the
asher/open-recent
branch
from
87d872e to
d7d1b82
Compare
| @@ -31,6 +31,28 @@ The `vscodessh` subcommand on the `coder` binary periodically flushes it's netwo | |||
|
|
|||
| Coder Remote periodically reads the `network-info-dir + "/" + matchingSSHPID` file to display network information. | |||
|
|
|||
| ## Testing | |||
There was a problem hiding this comment.
I wanted this section below the development section, but when I do that Prettier goes haywire and starts injecting spaces all over the place, leading to insanity like this:
- Use the "VS Code Desktop" button from a Coder dashboard.
- Manually open the link wit h `Developer: Ope n URL` from in
sid e VS Code.
- Use `code -- open-url <
link >` on the co mm and l ine.
The
link format i s ` v sc ode://co der
.cod er- r emote/ ope n?$ { qu ery}
` . For exam ple:
``
`bash
cod e --ope n -url
'vs c od e :/ /coder
. cod e r -r e mo
te/ope n ?u rl= d ev.cod e r .c om &own e r= my - u s e rn am e &w o r k s pa c e =my - ws& a ge nt=my-agent'
There was a problem hiding this comment.
I think all documentation should look like that. Make everything like House of Leaves
| workspaceAgent: string | undefined, | ||
| folderPath: string | undefined, | ||
| openRecent: boolean | undefined, |
There was a problem hiding this comment.
Any particular reason for not using optional parameters via ?:?
There was a problem hiding this comment.
Just to make sure it is specified. I find sometimes when I add new arguments, if I make it optional with ?: I can miss calls where I should have added it. But doing it this way forces me to check every function call since they will error without being passed a value.
Once that is done, I might change it to ?: after some consideration, but in this case since in all our calls to this function we have a value to pass, I did not bother.
There was a problem hiding this comment.
Yeah, that makes sense. I think that I've favored the optional syntax for my own convenience up 'til now, but I can see how forcing all the parameters to be explicit would be handy, especially for code stability
| // Remove recents that do not belong to this connection. The remote | ||
| // authority maps to a workspace or workspace/agent combination (using the | ||
| // SSH host name). This means, at the moment, you can have a different | ||
| // set of recents for a workspace versus workspace/agent combination, even | ||
| // if that agent is the default for the workspace. |
There was a problem hiding this comment.
Do we have any docs on how this works? The comment seems well-written, but I feel like I'm still not quite getting it because of missing context
There was a problem hiding this comment.
Nope, not that I know of. I am honestly not really sure how to even write the comment, I rewrote it several times and still feel like I am not really explaining it well.
The problem is that both of these go to the same place:
ssh coder-vscode--my-workspace.my-agent
ssh coder-vscode--my-workspace # opens my-agent agent because my-agent is the default or only one
But VS Code has no idea these are the same, they are different strings after all. So depending on which you use you could get a different set of recents.
When clicking from the dashboard we seem to use the second form (without the agent name).
I think probably what we should do is always append the agent name if it is missing, and never use the workspace-only form. Then this will not be a problem at all. But for now I just left the comment.
There was a problem hiding this comment.
And to further clarify, this only arises when using the sidebar in VS Code, where you can both connect to a workspace or directly to an agent.
| @@ -31,6 +31,28 @@ The `vscodessh` subcommand on the `coder` binary periodically flushes it's netwo | |||
|
|
|||
| Coder Remote periodically reads the `network-info-dir + "/" + matchingSSHPID` file to display network information. | |||
|
|
|||
| ## Testing | |||
There was a problem hiding this comment.
I think all documentation should look like that. Make everything like House of Leaves
Feels self-explanatory?
We can just use the selected value directly.
This covers the plugin side of #241. We will still need to make changes on the module side.