Skip to content
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

liquidprompt: init at 2018-05-21 #66429

Merged
merged 1 commit into from Sep 20, 2019
Merged

Conversation

Gerschtli
Copy link
Contributor

Motivation for this change

Add liquidprompt package at 2018-05-21. Unfortunately the last tag in the repo is a while back and missing features, so I took the rev of the most recent commit.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@alexarice
Copy link
Contributor

Is it possible to ask upstream for a tag?

@Gerschtli
Copy link
Contributor Author

Done!

@mmahut
Copy link
Member

mmahut commented Aug 25, 2019

@GrahamcOfBorg build liquidprompt

@mmahut
Copy link
Member

mmahut commented Aug 26, 2019

@GrahamcOfBorg build liquidprompt

Copy link
Contributor

@romildo romildo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the script installed at $out/share/{bash,zsh}/plugins/liquidprompt/? As far as I know, those are not standard locations, and it may make it difficult to start the script. I would rather install at $out/bin.

@romildo
Copy link
Contributor

romildo commented Sep 18, 2019

It would also be interesting to install the default configuration and theme files. They would serve as examples for the user willing to change configuration and theming.

@Gerschtli
Copy link
Contributor Author

As liquidprompt needs to be sourced instead of executed (see missing executable bit) this file definitly does not belong into $out/bin.

What files would you add to the package and where? Personally, I don't really see the advantages of putting default configs into the package as the majority will define their own config. And if you need a reference, you will be better of with reading the README of liquidprompt.

@romildo
Copy link
Contributor

romildo commented Sep 18, 2019

According to https://superuser.com/questions/176783/what-is-the-difference-between-executing-a-bash-script-vs-sourcing-it both execution and sourcing of a bash shell script uses PATH to find the script file. The difference is only in the environment used: execution creates a new environment, which is discarded when the script finishes, and sourcing does not create one, running the script in the same process which sourced it.

It is desirable that the script be placed in a directory in PATH, making it easier to be found when sourced.

Currently I am sourcing it with something like

. /nix/store/nbxjwa5vz6vgvwhxjvn909am5ly9payw-liquidprompt-unstable-2018-05-21/share/bash/plugins/liquidprompt/liquidprompt

which is not very user friendly and will change in the next rebuild if the source or any dependency changes.

If it can be found in PATH, it could be just:

. liquidprompt

It does not need to be executable, though. That would be enough to prevent it from being executed instead of sourced.

Other distributions like Arch (AUR) installs it at /usr/bin.

@romildo
Copy link
Contributor

romildo commented Sep 18, 2019

I would install liquidpromptrc-dist and liquid.theme to $out/share/doc. Maybe README.md and demo.png too.

@romildo romildo merged commit 9653933 into NixOS:master Sep 20, 2019
@Gerschtli Gerschtli deleted the add/liquidprompt branch September 20, 2019 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants