-
Notifications
You must be signed in to change notification settings - Fork 965
fix(desktop): stabilize Windows installer + beta 0.0.64 #1119
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
Closed
Closed
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
7b2f628
fix(desktop): improve Windows installer
AXGZ21 afdb06d
chore(desktop): update app icons
AXGZ21 7f2399c
fix(desktop): stabilize Windows installer assets
AXGZ21 e4a00f9
chore(desktop): bump version to 0.0.64
AXGZ21 ff95bc9
fix(desktop): restore @lydell/node-pty-win32-x64 files for Windows in…
PierrunoYT 4aa2254
fix(desktop): update default window icon path for packaged apps
PierrunoYT f16d0c9
feat(desktop): implement ringtone event subscription and refactor eve…
PierrunoYT 9cce4ec
fix(desktop): improve error handling for Windows sound playback
PierrunoYT 3bbbbcf
fix(desktop): enhance fallback handling for Windows ringtone playback
PierrunoYT 8b3eb0f
fix(desktop): add conditional check for desktop shortcut creation
PierrunoYT a91300b
fix(tabs): refine legacy pane migration logic
PierrunoYT 9e58140
fix(api): add user existence check in session verification and update…
PierrunoYT 78f3697
fix(desktop): enable npm rebuild for native modules during packaging
PierrunoYT 9756032
fix(desktop): enhance ringtone event handling with validation
PierrunoYT 2b69077
fix(desktop): streamline ringtone stop functionality for Windows
PierrunoYT File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
31 changes: 29 additions & 2 deletions
31
apps/desktop/src/lib/electron-app/factories/windows/create.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AXGZ21 what's the reason for using this package? I think i had to do this for onlook as well but can't recall exactly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question! The
@lydell/node-ptypackage is a fork of the originalnode-ptylibrary. Based on the change, there are a few likely reasons why this fork was used:Possible reasons:
node-ptydeals with pseudo-terminals, this fork might have better cross-platform support (Windows/macOS/Linux)To confirm the exact reason, it would be helpful to:
Did you encounter this same issue with Onlook, or are you just reviewing the change for context?
AI Generated by PierrunoYT
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Kitenite We use npm:@lydell/node-pty@^1.0.1 because it provides reliable prebuilt binaries (especially for Windows) for Electron, so terminal PTY spawning works and installer builds don’t fail on native compilation.
Also, keep only one node-pty entry in package.json (remove the duplicate).