Changes by icoco re: Desktop app #690
Conversation
There was a problem hiding this comment.
@icoco There's a lot going on here!
It looks like you needed to make some adaptations for your use cases of VZCode, but did not consider the changes as contributions to VZCode.
I'd be happy to incorporate your improvements to mainline VZCode. That would be beneficial to you, since you won't need to maintain a separate fork.
I would encourage you to review the contribution guidelines, open new issues for each thing you need, then open separate PRs that close each of the issues.
Certain things may only be relevant to your use case, in which case they can be put behind a feature flag.
Thanks for working on VZCode!
| if ( globalThis.wss){ | ||
| return ; | ||
| try{ | ||
| globalThis.connection.close(); |
There was a problem hiding this comment.
Why is globalThis required?
| ); | ||
| const connection = new Connection(socket); | ||
| globalThis.wss = socket; | ||
| globalThis.connection = Connection; |
There was a problem hiding this comment.
Why are these exposed in this way?
| import { Request } from "../utils/Request.js"; | ||
|
|
||
| // get the docId from the current url request paramter | ||
| export const getRequestDocId = function(){ |
There was a problem hiding this comment.
Whoah this is cool! I'd like to learn more about what is going on here. What is the doc id? What is this used for?
| @@ -0,0 +1,5 @@ | |||
| .AppShell { | |||
| const fetchDocStatus = reloadDocThen() | ||
|
|
||
| function AppShell() { | ||
| const docStatus = fetchDocStatus.read(); |
There was a problem hiding this comment.
docStatus is not used. What does .read() do here?
| @@ -9,6 +9,10 @@ import { json } from '@codemirror/lang-json'; | |||
| import { markdown } from '@codemirror/lang-markdown'; | |||
| import { html } from '@codemirror/lang-html'; | |||
| import { css } from '@codemirror/lang-css'; | |||
|
|
|||
| import { xml } from '@codemirror/lang-xml'; | |||
There was a problem hiding this comment.
Very nice! These could be separate PRs, I would accept these by themselves.
| @@ -29,7 +29,7 @@ export const Listing = ({ | |||
| name={name} | |||
| handleFileClick={handleFileClick} | |||
| handleFileDoubleClick={handleFileDoubleClick} | |||
| isActive={fileId === activeFileId} | |||
| isActive={fileId === globalThis.activeFileId } | |||
| @@ -0,0 +1,35 @@ | |||
| import process from 'node:process'; | |||
|
|
|||
| const wrapUnderSelectedFolder = function(shortName){ | |||
There was a problem hiding this comment.
What is this for? Looks like you are wanting to see the full path?
| } | ||
| } | ||
|
|
||
| export const openBrowser = function (url) { |
There was a problem hiding this comment.
Very nice cross platform browser opening!
Creating this PR to see the changes clearly from @icoco . Thank you for this!