Avoid looking at PS1 to determine user's preference,
and just explicity ask them.
Use a file to save the preference between invocations (e.g. nix develop)
Details
Details
- Reviewers
atul varun • abosh - Group Reviewers
Restricted Owners Package (Owns No Changed Paths) - Commits
- rCOMM04916a6cd2ae: [Nix] Use file to determine user preference
nix develop # should get prompted # if yes, prompt should change # if no, prompt should not change rm ~/Library/Caches/app.comm/enable-powerline exit nix develop # try other option
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| nix/start-powerline.sh | ||
|---|---|---|
| 16 ↗ | (On Diff #15633) | Should Powerline be capitalized? |
Comment Actions
Going to review this more carefully after weekly sync, but just wanted to make sure these changes are solely cosmetic?
Doesn't it make the nix environment a little less "deterministic" in that it introduces potential variability for different members of the team? Not sure it makes a practical difference but just wondering.
Comment Actions
Nix is about reproducable builds, not about forcing every dev to have the exact same settings for their shell
Comment Actions
Couldn't different shell settings potentially break reproducibility?
Like if one dev sets a shell option eg
shopt -s globstar
that would definitely change behavior of build scripts and break reproducibility.
These changes definitely seem "benign" and cosmetic and I'm not arguing against them, just figured we'd generally want to limit the amount of variability in environment.
Comment Actions
Couldn't different shell settings potentially break reproducibility?
This is true, but even if we didn't include the option to use Powerline it would still be true. Even if we're all on Nix, we can't prevent someone from changing their settings. Developers will inevitably have different shell settings or use different shells altogether. I myself use fish which is different from zsh and bash, but it's my responsibility to understand the differences and take into account that variability.
Your point is still valid. The globstar example is a good one because using ** in scripts is really common because it's so useful. Yet it can only be run if the globstar option is set. Thus in the future, we may run into issues with different shell configurations, but this script's purpose is just to change the prompt / colors a user sees. It shouldn't modify something that could break reproducibility.
@jon I'm curious for your input.
| nix/start-powerline.sh | ||
|---|---|---|
| 15 ↗ | (On Diff #15633) | I've seen this before in read commands, but why do we need to prevent backlashes here? I know a user could provide malformed or malicious input (that could contain backslashes) but we wouldn't match it anyways / do anything with the command based on our case statement, right? Or is adding -r just standard for read? |
| 16 ↗ | (On Diff #15633) | Can we hyperlink Powerline? Just Googling "Powerline" does not provide any immediately meaningful results (no Wikipedia sidebar, top result is not Powerline docs/website). More information is always better if we're modifying user experience. |
| 18–25 ↗ | (On Diff #15633) | case statements can be difficult to read. What version of bash are we using? If we're on version 3.2+, we can use an if-else: if [[ "$response" =~ ^([yY][eE][sS]|[yY])$ ]]
then
echo "1" > "${COMM_POWERLINE}"
else
touch "${COMM_POWERLINE}"
fiIf we're on bash 4.0+: response=${response,,} # Convert the response to lowercase
if [[ "$response" =~ ^(yes|y)$ ]]
then
echo "1" > "${COMM_POWERLINE}"
else
touch "${COMM_POWERLINE}"
fi |
| 28 ↗ | (On Diff #15633) | Could we just use -s here? From https://www.gnu.org/software/bash/manual/bash.html:
|
Comment Actions
Couldn't different shell settings potentially break reproducibility?
Nix has strong guarantees around building packages with it. Shell usage is a non-goal for nix.
Like if one dev sets a shell option eg
We've already had this with zsh's extGlob syntax and that things like nix run .#mariadb-up, the solution is to escape it nix run '.#mariadb-up'.
After nix develop is done, this is a non-issue because we are now in bash, but it's still odd.
that would definitely change behavior of build scripts and break reproducibility.
Not in a meaningful way, the shebangs will force the scripts to be executed by a particular shell. Currently we force bash on almost all of our scripts anyway.
These changes definitely seem "benign" and cosmetic and I'm not arguing against them, just figured we'd generally want to limit the amount of variability in environment.
Yea, but I would argue there's a difference between how a developer interfaces with tooling, and what tooling is present. The main purpose of leveraging nix is so that everyone has the same tooling, but I think it's best for developers to choose their own way to interface with it.
For example, I already have a pretty extensive bash setup, and would like to have it over powerline in my setups.
| nix/start-powerline.sh | ||
|---|---|---|
| 15 ↗ | (On Diff #15633) | I believe it's considered best practice, but every invocation of read I see includes it. So I think we should keep -r. |
| 16 ↗ | (On Diff #15633) | Yes, we should. |
| 16 ↗ | (On Diff #15633) |
I would like to avoid this, AFAIK, hyperlink support for terminals is very hit or miss on a given terminal. And I'm not aware of any standard way to do it. I would be find with just adding another line which has the full explicit link to the github readme, my terminal emulator is at least able to auto-hyperlink valid hyperlinks. Maybe punt on this and add it later, as I think it increases the scope of what is trying to be achieved here; and this is blocking other work like the binary cache. I created https://linear.app/comm/issue/ENG-1627 to track this. |
| 18–25 ↗ | (On Diff #15633) | The intention is that this can also be done with zsh or ancient versions of bash. We can't make assumptions about the shell until after nix develop, and even then would still like to avoid shell-specific patterns. case, although awkward, is pretty universally supported. |
| 28 ↗ | (On Diff #15633) | Yea, this would probably be better. |
Comment Actions
Even if we're all on Nix, we can't prevent someone from changing their settings. Developers will inevitably have different shell settings or use different shells altogether. I myself use fish which is different from zsh and bash, but it's my responsibility to understand the differences and take into account that variability.
You're able to use fish in a nix environment?
Comment Actions
You're able to use fish in a nix environment?
You would have to manually launch it after nix develop but you can. fish will inherit all of the relevant ENV vars needed.
Comment Actions
Does this stack fix the issue I was seeing where press-up-to-see-previous-commands wasn't working?
Comment Actions
Does this stack fix the issue I was seeing where press-up-to-see-previous-commands wasn't working?
I think that's a separate issue, as my terminal has the normal "up means go up in history" behavior.
In https://linear.app/comm/issue/ENG-1550#comment-854f3150 I mention that it's possible that your terminal is somehow put in "alternate console" mode. The fix for which is usually doing tput rmcup. I could forcefully run this in nix develop as this will likely have the desired behavior anyway, but might cause issues with things like tmux (haven't tried).
But how did my terminal ever get into the "alternate console" mode to begin with?
Pretty much any application which grabs the cursor will do this: less, more, vim, nano, etc. If the program doesn't exit cleanly then your console can be in this weird state of not being "released" back to the parent process correctly. If you're only getting this when doing nix develop, then that's very odd. My m1, intel mac and NixOS aren't having that behavior even when using Powerline. We can create another task, and I can try to troubleshoot it separately.