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

Support using colors from gnome terminal #53

Merged
merged 1 commit into from
Jul 16, 2021

Conversation

ccat3z
Copy link
Contributor

@ccat3z ccat3z commented Jun 20, 2021

Since a lot of Gnome Shell users use Gnome Terminal, I think it is useful to sync with the color profile of Gnome Terminal.

upload.mp4

@amezin
Copy link
Member

amezin commented Jun 20, 2021

Can this be just a button in preferences dialog (i. e. "Copy settings from GNOME Terminal")?

Unchecking the checkbox won't revert settings to the old values, as far as I can tell.

Continuous sync may look cool, but is it actually useful? How frequent do you change colors in GNOME Terminal?

@ccat3z
Copy link
Contributor Author

ccat3z commented Jun 23, 2021

You're right, I rarely change colors profile. I added the "Copy profile from GNOME Terminal" button at the bottom of the colors panel.

Copy link
Member

@amezin amezin left a comment

Choose a reason for hiding this comment

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

Everything except logError()s looks good.

Would be nice if it was possible to select the profile to import (button with a popup menu?) - but it is not a requirement.

prefs.js Outdated Show resolved Hide resolved
@ccat3z ccat3z requested a review from amezin June 25, 2021 05:56
prefs.js Outdated Show resolved Hide resolved
prefs.js Outdated Show resolved Hide resolved
prefs.js Outdated Show resolved Hide resolved
@ccat3z ccat3z force-pushed the gnome-terminal-colors branch 2 times, most recently from e6575b4 to 38e677f Compare July 15, 2021 12:58
@ccat3z ccat3z requested a review from amezin July 15, 2021 13:06
Copy link
Member

@amezin amezin left a comment

Choose a reason for hiding this comment

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

Looks good, only 2 cosmetic changes are necessary, and then I'll merge it. Also, please squash commits into one (I can do it during merge, but your signature will probably be lost this way). Thank you!

util.js Outdated Show resolved Hide resolved
prefs.js Outdated Show resolved Hide resolved
@amezin amezin merged commit 32be055 into ddterm:master Jul 16, 2021
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.

None yet

2 participants