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

feat: cross platform terminal settings #22

Merged
merged 4 commits into from
Sep 25, 2024

Conversation

hcavarsan
Copy link
Contributor

@hcavarsan hcavarsan commented Sep 24, 2024

updated how app handle terminal settings for docker exec. now user can choose from more terminal apps like gnome-terminal, konsole, alacritty, xterm, terminator, xfce4-terminal, cmd.exe, powershell.exe, terminal, iterm, and wezterm.

added a dropdown menu in the settings screen where user can pick preferred terminal app. it saves automatically, so when app run a docker exec command, it'll use the terminal selected.

relates: #12

macos:

Area.mp4

linux (fedora):

linux.mp4

@hcavarsan hcavarsan changed the title Cross plat terminal feat: cross platform terminal settings Sep 24, 2024
Copy link
Owner

@ropali ropali left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall a good & much needed PR. I have added some suggestions for improvement.

src-tauri/Cargo.toml Outdated Show resolved Hide resolved
src-tauri/src/commands/terminal.rs Outdated Show resolved Hide resolved
src-tauri/src/commands/terminal.rs Outdated Show resolved Hide resolved
src-tauri/src/constants.rs Outdated Show resolved Hide resolved
src-tauri/src/utils/terminal.rs Outdated Show resolved Hide resolved
src/components/Screens/SettingsScreen.jsx Show resolved Hide resolved
src-tauri/src/commands/terminal.rs Outdated Show resolved Hide resolved
@hcavarsan
Copy link
Contributor Author

thanks, that makes sense. i’m already working on the changes and will push the adjustments soon (probably by tomorrow)

@hcavarsan
Copy link
Contributor Author

hcavarsan commented Sep 25, 2024

Hey @ropali, I made some changes based on the review comments. Can you check them? I think the code is now better, more flexible, and simpler.

linux:
https://github.com/user-attachments/assets/720d16fb-f92b-4d0c-9716-3f1215bed539

macos:
https://github.com/user-attachments/assets/d3fb9c44-0f40-4010-95b3-a9a5de70d1c4

@ropali
Copy link
Owner

ropali commented Sep 25, 2024

Hey @ropali, I made some changes based on the review comments. Can you check them? I think the code is now better, more flexible, and simpler.

linux: https://github.com/user-attachments/assets/720d16fb-f92b-4d0c-9716-3f1215bed539

macos: https://github.com/user-attachments/assets/d3fb9c44-0f40-4010-95b3-a9a5de70d1

I took a quick look and changes look good so far. please allow me some time to review it thoroughly.

@ropali ropali merged commit 2397788 into ropali:main Sep 25, 2024
@hcavarsan hcavarsan deleted the cross-plat-terminal branch September 25, 2024 17:02
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.

2 participants