Gogh 2.0 #160

phenonymous · 2018-11-30T19:32:39Z

Gogh 2.0

This PR started with me wanting this to work for tilix but ended up with a much bigger update. I'll outline what this update contains below.

List of changes

General changes

Added support for tilix
Added support for guake 3
Added logo before list of themes
Added ability to change url to forked url
Added ability to view real theme colors if terminal support truecolor
Added a script to view all themes without applying the themes
Added a pre-check if a profile already exists, this is to avoid installing multiple profiles of the same theme
Added two new themes, google-light and google-dark
Added option to apply all themes in one go
Added a progressbar to view progress made if users chooses more than 5 themes
Removed curlsource from all scripts where applicable. This got replaced by bash -c "$(curl <url>)" instead
Removed all source commands and replaced with bash -c <file> for remote files and bash for local files
Changed all [ tests to [[
Changed all 4 space indentations to 2 space indentations
Changed all curl and wget commands to include silent option. This is to avoid cluttering the screen

General theme changes

Moved gogh_colors to apply-colors.sh
Added export before all variables since all source commands got replaced

gogh dot sh

Changed function definition of set_gogh
Moved terminal check from apply-colors.sh
Changed the way terminal check works by looping until $PPID is no longer a shell.
Added a subsection that exports some variables that are relevant for tilix

apply-colors dot sh

This script got an complete overhaul - more info further down

Added check to see if the script has all the tools it needs in the beginning of the script
Defined functions for code that was repeated multiple times
Removed the long terminal if statement and replaced it with case
Defined functions to apply theme for various terminals
Added a dry run that exists the script before applying any themes if test/print-themes.sh was invoked

Tilix support

This update adds support for tilix terminal. tilix support both profiles and color schemes. A code subsection in gogh.sh checks if current terminal is tilix and asks the user if they want to use color schemes instead of profiles. If they do a temporary directory is made and all color schems gets copied over to that folder. When all themes has been processed all color schemes are copied over to the default tilix color scheme directory $HOME/.config/tilix/schemes and the temporary directory is deleted.
If the user aborts before all themes has been processed, the temporary directory gets deleted and no color schemes are copied.

Overhaul of apply-colors.sh

The reason why I completely changed this script is because I found the old script cumberstone to troubleshoot. Instead of if statements I use a case statement instead and from there calls functions that are applicable for the current terminal.
These functions are

apply_cygwin
apply_darwin
apply_guake
apply_elementary
apply_gtk

With this change I was able to merge pantheon-terminal and elementary-terminal to one function, apply_elementary. Likewise apply_gtk is a merge of gnome-terminal, mate-terminal and tilix. This makes it much easier to add any gtk-vte based terminal in the future.

Issues

Fixed Error, no saved profiles found! #149, Gogh does not work, if there is no Default profile #137 by adding a fallback default if no default profile is found.
This can happen even if a default profile exists
Fixed error: 5:expected value #136 by correcting the implementation of how mate-terminal was handled
Fixed Doesn't working Debian #63, Not Working on Ubuntu #41 altough I can not give any specifics on how. It's just an side effect of this update. Verified by successfully applying themes on the same setup as referenced in the issues.

Bug fixes not listed in issues

Fixed a bug where option 08 and 09 would throw an "Invalid option"
Fixed a bug where gnome-terminal version 3 and above would not work because they changed the terminal name identifier to gnome-terminal- note the hyphen
Fixed a bug where elementary-terminal in elementary OS 5.0 Juno would not work because they changed terminal name identifier to io.elementary.t

Verification

This is a non-breaking update. I have merged all the latest commits into this PR so you would be able to auto-merge.

The following table illustrates which terminals and OS's I have verified this to be working on.

Terminal	Operating system
Gnome terminal v2	Ubuntu 14.04
Gnome terminal v3	Ubuntu 18.04
Gnome terminal v3	Debian 9.6
iTerm2	macOS High Sierra
iTerm2	macOS Mojave
mate-terminal	Ubuntu 18.04
tilix	Ubuntu 18.04
Guake 3	Ubuntu 18.04
babun (mintty)	Windows 10 1803

The verification tests inluded the following:

Tested both locally (from cloned repo) and remote (using the URL mentioned above)

Install themes from gogh.sh
Install theme directly from a given themes/theme.sh
Print all themes from test/print-themes.sh

I have tried to give an as detailed PR as possible without writing an essay. If you have any questions please do ask.

Keep up to date with upstream

Changed one liner command to a more simpler one

notlmn · 2018-11-30T19:47:07Z

Added two new themes, google-light and google-dark

Can you do this in a separate PR, that would make this PR a little bit easy to review. One step at a time.

Removed curlsource from all scripts where applicable. This got replaced by bash -c "$(curl <url>)" instead

Removed all source commands and replaced with bash -c <file> for remote files and bash for local files

Wouldn't this cause problems for people who do not use bash as their main shell?

Added export before all variables since all source commands got replaced

Make sure that you do not pollute global variable space with all the exported variables, make sure to unset them.

test/test.sh

themes/3024-day.sh

phenonymous · 2018-11-30T21:12:46Z

Added two new themes, google-light and google-dark

Can you do this in a separate PR, that would make this PR a little bit easy to review. One step at a time.

I will make a separate PR for this once this PR has been merged so to follow the new theme format.

Removed curlsource from all scripts where applicable. This got replaced by bash -c "$(curl <url>)" instead

Removed all source commands and replaced with bash -c <file> for remote files and bash for local files

Wouldn't this cause problems for people who do not use bash as their main shell?

It will not. Consider this

If you call an executable script without specifying which shell (i.e ./script versa bash script) that does not have any shebang will use the current shell as the interpreter
If you call an executable script without specifying which shell (i.e ./script versa bash script) that do have a shebang will use the specified interpreter in the shebang regardless of the shell that called it.

Now if you call a script by specifying which shell to use i.e bash script will use that as the interpreter of the script regardless of what your current shell is if the script does not have any shebang.

Since all script files do use a shebang that specifies bash as the interpreter - all scripts will work as long as the user have bash installed, regardless of what your main shell is

Added export before all variables since all source commands got replaced

Make sure that you do not pollute global variable space with all the exported variables, make sure to unset them.

This is a good point. I will fix accordingly

…tion

gogh.sh

katerynarieznik · 2018-12-01T15:58:45Z

@phenonymous His is huge and awesome 😲🤩👏
Looking forward to seeing your google themes 👍

katerynarieznik · 2018-12-01T16:01:08Z

also @notlmn thanks for your review 🙇‍♀️

Fix suggestions in #160

Mgldvd · 2018-12-01T19:19:38Z

Hi @phenonymous!
Thanks for this. Another amazing PR.
Whenever I see contributions to this repository I am speechless to see how this repository continues to evolve and for all the work you put into it. .
@notlmn thanks too for your review.

notlmn · 2018-12-01T20:29:36Z

@phenonymous Thanks for the PR, I really like rewrites, they always bring something new.

@Mayccoll @rieznik Glad I could be some help.

notlmn · 2018-12-01T20:29:53Z

And I'd suggest from now on, to squash-commit any PR's. Because there is no point importing all the intermediary commits in a PR into the repo, and the original commits are still available in the PR anyway. And again, that's just what I think, and only a suggestion.

Thanks.

selfup · 2018-12-04T00:37:00Z

Wow this is awesome! 🎉

phenonymous · 2018-12-04T16:20:11Z

@notlmn Thanks for your review and for giving me a better understanding of git
@Mayccoll Thanks for this repository, you are my inspiration for starting bashhacking in the first place

Gogh 2.0 🎉 🎉 🎉 🎉 Another amazing PR thank you very much

Fix suggestions in Gogh-Co#160

phenonymous added 26 commits November 5, 2018 11:28

Major change

dfa6c56

Added tilix support

d1eafe6

Major update with tilix support

a453f6a

Update gogh.sh

b54f0cb

Update gogh.sh

0b4c843

Update gogh.sh

96c6098

Update gogh.sh

37e58cb

Update gogh.sh

e3c9f1d

Update gogh.sh

c5c84bf

Major update with tilix support

b4aae6e

Major update with tilix support

88f07d5

Update gogh.sh

f4f280d

Update gogh.sh

edd964b

Update gogh.sh

b2f7a80

Major update with tilix support

c60eb20

Major update with tilix support

db039c6

Major update with tilix support

61fb958

Merge remote-tracking branch 'upstream/master'

553adf3

Keep up to date with upstream

Add support to show all themes without applying

3e0fffd

Major update with tilix support

0ef494e

Major update with tilix support

75ad420

Showcase themes without applying the theme

e8195e9

Major update with tilix support

851ac59

Major update with tilix support

cd200b7

Change to 'bash -c' instead of 'eval'

e1dc74f

Update README.md

ae3ff8d

Changed one liner command to a more simpler one

notlmn reviewed Nov 30, 2018

View reviewed changes

test/test.sh Outdated Show resolved Hide resolved

notlmn reviewed Nov 30, 2018

View reviewed changes

themes/3024-day.sh Outdated Show resolved Hide resolved

phenonymous added 4 commits November 30, 2018 23:02

Add new line to EOF

5bddddd

Make sure exported variables get unset to avoid global variable pollu…

01f998e

…tion

Cleanup code

ef624d3

Add google themes in separate PR

8043e61

notlmn suggested changes Dec 1, 2018

View reviewed changes

gogh.sh Show resolved Hide resolved

gogh.sh Show resolved Hide resolved

Mgldvd merged commit d03a597 into Gogh-Co:master Dec 1, 2018

phenonymous mentioned this pull request Dec 1, 2018

Fix suggestions in #160 #162

Merged

Mgldvd pushed a commit that referenced this pull request Dec 1, 2018

Merge pull request #162 from phenonymous/master

683bf20

Fix suggestions in #160

z-nexx pushed a commit to z-nexx/Gogh that referenced this pull request Sep 12, 2021

Merge pull request Gogh-Co#160 from phenonymous/master

db3e92a

Gogh 2.0 🎉 🎉 🎉 🎉 Another amazing PR thank you very much

z-nexx pushed a commit to z-nexx/Gogh that referenced this pull request Sep 12, 2021

Merge pull request Gogh-Co#162 from phenonymous/master

b89132b

Fix suggestions in Gogh-Co#160

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gogh 2.0 #160

Gogh 2.0 #160

phenonymous commented Nov 30, 2018 •

edited

Loading

notlmn commented Nov 30, 2018

phenonymous commented Nov 30, 2018

katerynarieznik commented Dec 1, 2018

katerynarieznik commented Dec 1, 2018

Mgldvd commented Dec 1, 2018 •

edited

Loading

notlmn commented Dec 1, 2018

notlmn commented Dec 1, 2018

selfup commented Dec 4, 2018

phenonymous commented Dec 4, 2018

Gogh 2.0 #160

Gogh 2.0 #160

Conversation

phenonymous commented Nov 30, 2018 • edited Loading

Gogh 2.0

TOC

List of changes

General changes

General theme changes

gogh dot sh

apply-colors dot sh

Tilix support

Overhaul of apply-colors.sh

Issues

Bug fixes not listed in issues

Verification

Tested both locally (from cloned repo) and remote (using the URL mentioned above)

notlmn commented Nov 30, 2018

phenonymous commented Nov 30, 2018

katerynarieznik commented Dec 1, 2018

katerynarieznik commented Dec 1, 2018

Mgldvd commented Dec 1, 2018 • edited Loading

notlmn commented Dec 1, 2018

notlmn commented Dec 1, 2018

selfup commented Dec 4, 2018

phenonymous commented Dec 4, 2018

phenonymous commented Nov 30, 2018 •

edited

Loading

Mgldvd commented Dec 1, 2018 •

edited

Loading