-
-
Notifications
You must be signed in to change notification settings - Fork 156
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
fish: fix base16-fish causing startup issues with tmux #503
Conversation
Upstream issue: tomyun/base16-fish#7 Closes danth#488
@@ -7,6 +7,7 @@ let | |||
in '' | |||
source ${theme} | |||
|
|||
# See https://github.com/tomyun/base16-fish/issues/7 for why this condition exists |
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.
I think ideally the reasoning is written out in text here. What if the project eventually gets migrated off of Github or if Github goes down?
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.
I think ideally the reasoning is written out in text here.
Do you mean we should inline the explanation rather than delegating to an external source? I also prefer this, but I don't particularly mind in this case.
Feel free to open a PR replacing the comment with a concise and comprehensive summary of the problem.
What if the project eventually gets migrated off of Github
Hopefully, they would kindly link to the new location.
or if Github goes down?
In that case, NixOS might go down for a while 👀
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.
Do you mean we should inline the explanation rather than delegating to an external source?
Yes.
Hopefully, they would kindly link to the new location.
I highly doubt that would happen. Comments age like raw fish.
Anyway, just a suggestion. I probably won't make the effort to update a comment.
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.
If base16-fish
disappears / is moved we would have to migrate the entire module to a different solution anyway since users would no longer be able to fetch the flake input
Remove the obsolete $base16_theme check which was incorrectly taken from [1] as $base16_theme is unused in Stylix. [1]: tomyun/base16-fish#7 (comment) Closes: danth#538 Fixes: 94d7029 ("fish: fix base16-fish causing startup issues with tmux (danth#503)") Link: danth#539 Tested-by: Donovan Glover <donovan@dglover.co> Approved-by: Donovan Glover <donovan@dglover.co> Reviewed-by: NAHO <90870942+trueNAHO@users.noreply.github.com>
Upstream issue: tomyun/base16-fish#7
Closes #488
Tested and works as expected