Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vscode remote support #18

Merged
merged 2 commits into from
Jan 13, 2021

Conversation

flaw
Copy link
Contributor

@flaw flaw commented Nov 24, 2020

Fixes #17

Testing has been pretty minimal, it works for me in code-insiders 1.52.0.

I noticed though that at the moment it only reports the project as the name of the workspace folder, seems that workspaceFolder.uri.path actually returns just the folder name instead of a full path. There is uri.fsPath that might return something more aligned with what was linked in #17 but I feel that's another change if that's wanted/needed.

Dashboard:
image

flaw added 2 commits November 24, 2020 19:26
Instead of getting a file path and constructing that into
a Uri, get the file Uri directly from activeTextEditor.document.uri.
The VSCode APIs are remote aware and this returns a Uri that the other
API methods understand even when using a remote connection.
Extensions default to running on the remote host when using
the VSCode Remote extensions, this change forces the extension to
run on the client side instead.
This is required so that we can actually open a connection towards
aw-server that is listening on localhost.
@yemzz
Copy link

yemzz commented Dec 17, 2020

Please merge this, heavily needed functionality

@ErikBjare
Copy link
Member

I don't know much about this watcher (nor VSCode) but looks like this is in a mergeable state?

@johan-bjareholt Please review and merge at will.

@johan-bjareholt
Copy link
Member

I noticed though that at the moment it only reports the project as the name of the workspace folder, seems that workspaceFolder.uri.path actually returns just the folder name instead of a full path. There is uri.fsPath that might return something more aligned with what was linked in #17 but I feel that's another change if that's wanted/needed.

That does not seem to be the case for me. For example, for me I have a project value that is the following: /home/johan/Programming/activitywatch/aw-watcher-vscode. I'm on Linux though, I'd guess that it either differs depending on OS or depending on if it's a remote file.

This comment seems to document the difference between path and fsPath well: https://github.com/microsoft/vscode-uri/blob/0a9cabd/src/uri.ts#L193

According to that fsPath might use backslash instead of slash which aw-webui cannot handle. But according to that it seems that path should also point out the full path. So I don't think using fsPath is an option.

Regardless, it's not the end of the world if only the workspace folder name is set, the only issue is in case someone has the same name on the folder in two projects on different paths which I assume isn't a very common use-case.

@johan-bjareholt
Copy link
Member

johan-bjareholt commented Jan 6, 2021

Also, if you look at "top file activity" in the screenshot it's just supposed to display the filename and not the whole path, so something is not working correctly. It's probably because it uses backslashes while we only support ordinary slashes. Why that happens though I have no idea.
https://docs.activitywatch.net/en/latest/buckets-and-events.html#app-editor-activity

@ErikBjare
Copy link
Member

@johan-bjareholt Regarding the backslashes, it seems like that's the behavior on Windows already. See the screenshot in #10 (comment)

We should probably just accept that some paths will be Windows-style and fix the handling of it wherever it goes wrong (probably the web UI?).

@johan-bjareholt
Copy link
Member

@ErikBjare Aha, that makes sense. I remember that the code cutting the filename from the path is in the web-ui, so that's where it needs to be fixed.

@ErikBjare
Copy link
Member

@johan-bjareholt Nothing else blocking merge of this PR?

@johan-bjareholt
Copy link
Member

No I don't think so.

@flaw Thanks for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Does not work using Remote - WSL (and presumably the other VSCode Remote variants)
4 participants