tinted-theming / tinted-shell

Base16 and Base24 for Shells
https://github.com/tinted-theming/home
MIT License
65 stars 217 forks source link

profile_helper.sh sets the theme when it is sourced even if the theme did not change #23

Closed maksut closed 1 year ago

maksut commented 1 year ago

Describe the bug

Using the recommended .zshrc configuration:

# Base16 Shell
BASE16_SHELL_PATH="$HOME/.config/base16-shell"
[ -n "$PS1" ] && \
  [ -s "$BASE16_SHELL_PATH/profile_helper.sh" ] && \
    source "$BASE16_SHELL_PATH/profile_helper.sh"

But every time profile_helper.sh sourced via .zshrc, it resets the theme. Meaning, it recreates the theme soft link and executes the hooks. One of my tools (waybar) refreshes itself when the underlying (hook) link is recreated. This means whevener a terminal is opened, the tool resets with a flicker. A bit annoying.

Expected behavior

No need to create the soft links and executing hooks when the theme did not change.

System

Operating system: archlinux

Terminal: foot with zsh

Base16 tool waybar

Additional context

I've put this little hack in set_theme function in profile_helper.sh just before the soft link creation:

  # Does the symlink already exists with the same target?
  if [ "`readlink $BASE16_SHELL_COLORSCHEME_PATH`" -ef "$script_path" ]; then
      return 0 # then no need to do recreate the link and execute the hooks
  fi
JamyGolden commented 1 year ago

@maksut I've created a PR to fix this. Can you checkout the branch jamy/bugfix/reload-existing-theme and see if that fixes your issue? https://github.com/tinted-theming/base16-shell/pull/35

maksut commented 1 year ago

Looks like I've commented on the PR instead of here. So I'll repeat it here:

Sorry, but no it doesn't fix it.

Is this condition correct?

  if [[ "$theme_name" == "$current_theme_name" && "$theme_name" == "$BASE16_THEME" ]]; then
    return 0
  fi

Maybe that needs to be an OR instead of AND. In my testing $BASE16_THEME was not set (or empty not sure).

JamyGolden commented 1 year ago

Let's move the conversation to the PR but I'll leave the issue open for now.