-
Notifications
You must be signed in to change notification settings - Fork 179
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
Automatically detect light terminal on initialization. Implements #150. #151
base: master
Are you sure you want to change the base?
Conversation
I also implemented Apple Terminal light mode detection. One thing to keep in mind, though, is that this method sets the color scheme when the shell is first initialized. If dark mode changes after the shell has been initialized, Mcfly will display the wrong color scheme. This is partly why I had first suggested implementing this logic directly in the Rust codebase so that the checks can be made on each invocation of Mcfly. This situation can happen if people's color scheme changes automatically at day/night time, for example. Would you consider implementing this in the main codebase, @cantino? |
That's a good point. I guess it's a bit annoying to have to call things like |
Totally up to you — I think it would be really nice to have it within the Rust codebase so it's dynamic/set upon each invocation (I have a tendency to leave Terminal windows open for days, so this would be really nice), but I agree with the annoyingness of having it in the Rust codebase too. Also, I just timed how long I'd love to implement this myself, but unfortunately I have yet to learn Rust! |
It'd be great if we could only rely on env variables but it's not the case. However, that slight slow down would only happen on Apple Terminal, and if I suggest:
The "magic" would only happen when |
How do we know when we're in that terminal and need to shell out to |
See my implementation here: 55554be |
What do you think about this in relation to #156? |
In my opinion, Mcfly should always try to do the right thing automatically without the need for any configuration, which is why I think it'd be wise to have something directly in Mcfly to try to automatically detect light mode, especially since it's rather trivial to do it effectively in 95% of cases. As an example, I don't need to configure |
I think having this in the shell scripts is simpler for the time being, but eventually could be pulled into the Rust code. Is this PR ready to merge? (@dmfay & @SafariMonkey appreciate a review on this too if you can.) |
I would suggest keeping the previous value (if set) using something like Otherwise, it looks fine to me, but I'm far from an expert. |
there should exist a standard for all OS to return if it is using dark/light mode |
No description provided.