Repository navigation
Conversation
|
@Kechos23 is attempting to deploy a commit to the BS Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
ThisIs-Developer
left a comment
There was a problem hiding this comment.
Thanks again for this contribution. I’ve already tested the feature and shared my detailed requested updates in this comment:
#262 (comment)
Please follow the points mentioned there before the final update. Take your time with the changes, as some of them involve the Workspace structure and linked-file storage behavior.
|
Thanks for testing the feature and for the detailed suggestions. I have addressed the seven review points:
Related improvements include explicit rescans restoring removed links, background refreshes preserving open context menus, Save/Save As with Markdown filenames and independent copies, and one-way linked-to-Vault conversion through the existing HTML drag-and-drop mechanism. The update passed 29 unit tests, 95 targeted Chromium tests, and the static smoke check. A separate review executable was built using the project's unchanged official Neutralino runtime. Native manual validation is currently Windows-focused; macOS/Linux remain unverified. Browser/Docker controls are guarded and their ordinary workflows are covered by the regression tests. The custom Windows native-drop and single-instance experiments remain separate. This PR does not need them. Please let me know if you would prefer any of the related additions split out or reduced in scope. |
ThisIs-Developer
left a comment
There was a problem hiding this comment.
@Kechos23 Hi, thank you again for working through all of the previous points. I tested the latest version and I really like how the Linked Workspace feature is coming together. The previous requested updates look much better now.
I noticed a few smaller UI/workflow items, and I also checked a few edge cases against the current implementation.
1. Linked Workspace right-click menu
When I right-click Workspace (Vault), then a dropdown option shows with create new files/folders. I think the Linked Workspace should also have the same dropdown options.
It could contain:
- Link Markdown files
- Link Markdown folder
2. Welcome to Linked Workspace icon
The Welcome to Linked Workspace document currently uses the normal file icon. I understand that technically it is an internal document and not an external linked file, so the current implementation makes sense.
However, visually I think it would look more consistent if it used the Linked Workspace file/symlink icon as well, since the document belongs specifically to that workspace. Please share your opinion about this point with me.
3. Preserve linked-folder hierarchy inside Linked Workspace recovery storage
The Explorer UI correctly shows the linked folder hierarchy. For example, in my test the linked folder appears as its own folder and its files are nested underneath it.
However, when I inspect the file manager Markdown Viewer Vault, all linked recovery documents are stored directly under the Linked Workspace folder. The original linked-folder hierarchy is not represented there.
This would make recovery storage much easier to understand and inspect manually.
4. Confirmation before Save modifies the original linked file
I understand the current Linked Workspace design better now: edits are stored as recovery drafts, while Save / Ctrl+S explicitly updates the original external file. I think that behavior itself is reasonable.
My concern is accidental use. A user may open a linked document, edit it, press Ctrl+S out of habit, and not immediately realize that the original filesystem file will be changed.
Could we add a confirmation box (same UI as "reset-confirm-modal") the first time the user saves back to an original linked file?
Something like:
Save changes to the original linked file?
This will update the original file at:
C:\...\example.md
Cancel | Save to original
With a checkbox such as:
Don't show this warning again
Once the user understands the behavior, they can disable the confirmation permanently. I think this would make the feature safer without changing how Linked Workspace fundamentally works.
IMP NOTE
I personally haven’t reviewed the code deeply enough to confirm whether there are any potential data-loss or safety-related edge cases, so as an extra precaution, I asked Codex to review the implementation. It came back with the following four points:
| Priority | Codex Finding | Location |
|---|---|---|
| P1 | Check current editor text before refreshing linked files | script.js:11596–11601 |
| P1 | Preserve local drafts when a missing source returns | script.js:11613–11618 |
| P2 | Register watchers for links created by Save As | script.js:24997–25002 |
| P2 | Apply folder exclusions during bulk deletion | script.js:11809–11814 |
I’m not sure whether these are actual issues or just edge cases identified by Codex. If you could please check or reproduce these code paths and confirm that there is nothing to worry about, that would be completely fine as well.
I mainly wanted to raise them as a safety precaution before merging, especially for anything that could potentially affect users' files or local drafts.
If any of these points are valid, it would be really helpful if you could address them too.
Thanks a lot, man, for all the work you’ve done on this. I really appreciate it, and I really like the direction this feature has taken. I just want to make sure everything is safe and stable so we can get this merged as soon as possible. 👍
Love your work! ❤️
c09fdc7 to
9e8b706
Compare
|
Thanks for the detailed review. I have separated the shared persistence fixes from this desktop feature and rebuilt the proposal on that baseline.
The four safety notes are also covered: live primary/split drafts are captured before refresh; returning missing sources do not overwrite dirty drafts; Save As links receive watchers; and bulk removal persists exclusions before removing records. Regression tests cover success, cancellation, changing state during confirmation, persistence failure and partial folder operations. Additional related improvements include folder conversion/export, sidebar conversion using the existing HTML drag mechanism, less crowded source-path presentation, status-icon consistency and layout-independent saving. Remaining native atomicity limits are documented rather than represented as fully solved. If the desktop feature direction is not suitable for the project, I am happy to narrow or withdraw the feature proposal. The shared safety baseline is now proposed separately in #279; the description links the incremental Linked Workspace diff for easier review. |




Desktop Linked Workspace: original files, folder workflows and draft protection
This updates the reviewed Linked Workspace proposal with the requested hierarchy, actions and safety behavior. It depends on the separately proposed shared persistence/reliability changes; those are not presented as defects introduced by Linked Workspace.
User-facing behavior
file-symlink/folder-symlinkfor real linked sources, keeping normal Vault icons unchanged..mdwhen needed and proposes the document name. A Vault export keeps its Vault document and focuses the new linked document; a linked Save As keeps both linked documents.book-open-textfor both guides. Update only the Linked Workspace guide.Safety and failure handling
Platform and limits
Linked-source actions require desktop native APIs and are hidden/disabled in the web app, including Docker-served deployments. Browser recovery content remains usable without reconnecting local paths. Native development/manual evidence is Windows-based; macOS/Linux native behavior is not claimed verified.
This branch uses the official Neutralino 6.5.0 runtime/client pins. Standard external HTML file drops remain Vault-copy imports. Native path drops, single-instance integration and the experimental pointer fallback are proposed separately in draft #269.
Scan limits: 10,000 Markdown files, 40,000 entries, 2,000 directories, depth 32, 20 MiB per Markdown source, 200 MiB staged content and a 60-second scan budget. Recovery is not a backup. Native APIs do not provide an atomic compare-and-write/no-overwrite guarantee, so a race with another process cannot be completely eliminated by application-side checks.
Validation
Review dependency
Depends on shared safety/reliability PR #279. Because this cross-fork PR still targets upstream
main, its full diff currently includes that baseline. For the Linked Workspace-only changes, use the incremental comparison. After #279 merges, I will rebase onto the resulting upstream baseline.