home-manager icon indicating copy to clipboard operation
home-manager copied to clipboard

starship settings require ordered attrsets

Open arcnmx opened this issue 2 years ago • 11 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Issue description

Starship uses an IndexMap for its module settings that use dictionaries/attrsets so that they can be processed in declaration order. This doesn't translate well to nix, which doesn't really have ordered attrsets - and so certain settings are impossible to set properly via programs.starship.settings.

Maintainer CC

No response

System information

host os: `Linux 5.15.4, NixOS, 22.05 (Quokka)`
version: `nix-env (Nix) 2.3.16`

arcnmx avatar Nov 29 '21 22:11 arcnmx

Hi,

Do you have an example setting that cannot be set currently with Home Manager?

berbiche avatar Dec 02 '21 06:12 berbiche

Specifically - because starship requires that values be set in the order they should be processed - you cannot set...

[directory.substitutions]
"/a/b" = "b"
"/a" = "a"

because in nix, sets are always sorted by keys, so...

{
  directory.substitutions = {
    "/a/b" = "b";
    "/a" = "a";
  };
}

... results in an incorrect configuration:

[directory.substitutions]
"/a" = "a"
"/a/b" = "b"

and the module has no extraConfig that could be used to get around this limitation :<

arcnmx avatar Dec 02 '21 17:12 arcnmx

Pretty unfortunate 🙁 May be worth adding an extraConfig of type.lines to cope with such situations.

rycee avatar Dec 03 '21 00:12 rycee

@arcnmx, @berbiche and @rycee, could format generators for DAGs be written, solving this issue for other modules as well? A toJSON function for DAGs would have to be implemented (see https://github.com/NixOS/nixpkgs/blob/master/pkgs/pkgs-lib/formats.nix), but that seems to be everything (other than maintaining the generators :slightly_smiling_face:).

Technically, JSON keys do not have an order, but remarshal (used by the generators) can preserve the order in its input JSON with the -p or --preserve-key-order flag.

rcerc avatar Dec 13 '21 13:12 rcerc

Thank you for your contribution! I marked this issue as stale due to inactivity. Please be considerate of people watching this issue and receiving notifications before commenting 'I have this issue too'. We welcome additional information that will help resolve this issue. Please read the relevant sections below before commenting.

If you are the original author of the issue

  • If this is resolved, please consider closing it so that the maintainers know not to focus on this.
  • If this might still be an issue, but you are not interested in promoting its resolution, please consider closing it while encouraging others to take over and reopen an issue if they care enough.
  • If you know how to solve the issue, please consider submitting a Pull Request that addresses this issue.
If you are not the original author of the issue

  • If you are also experiencing this issue, please add details of your situation to help with the debugging process.
  • If you know how to solve the issue, please consider submitting a Pull Request that addresses this issue.
Memorandum on closing issues

Don't be afraid to manually close an issue, even if it holds valuable information. Closed issues stay in the system for people to search, read, cross-reference, or even reopen – nothing is lost! Closing obsolete issues is an important way to help maintainers focus their time and effort.

stale[bot] avatar Mar 24 '22 22:03 stale[bot]

Enjoy my horrible implementation of programs.starship.extraConfig, meaniebot.

(fwiw, a DAG approach would be quite neat)

arcnmx avatar Mar 25 '22 18:03 arcnmx

I'm thinking that perhaps something like

{
  directory.substitutions = literalExpression ''
    "/a/b" = "b"
    "/a" = "a"
  '';
}

may be interesting?

rycee avatar Mar 25 '22 19:03 rycee

eh a DAG is what I'd call interesting, that's a more boring approach :stuck_out_tongue: but sure, it could solve this!

(implementation might be a little complicated, plus unlike JSON there isn't necessarily such thing as a ubiquitous TOML literal value since the syntax is somewhat context-sensitive, you might have to disambiguate by saying literalTableKeyValues or something, but it can be hacked together regardless - and really depends on how much you want to integrate with the conversion process or just collect the non-standard literals to be appended after conversion of all other keys)

arcnmx avatar Mar 25 '22 19:03 arcnmx

I wrote a toJson function for DAGs and some functions that use remarshal to convert DAGs to files of other formats (e.g. TOML and YAML). The code is here. A problem I encountered was finding the right place for the to<format>File functions because I didn't think they should be placed in modules/lib, as they depend on the Nix package set. I extended the set in modules/default.nix and other files it appears in with another one in modules/pkgs/default.nix that includes the format generators. However, I am not sure if there is a more idiomatic way to avoid keeping these functions in modules/lib, so I was hoping to hear your thoughts on this.

rcerc avatar Apr 03 '22 14:04 rcerc

Thank you for your contribution! I marked this issue as stale due to inactivity. Please be considerate of people watching this issue and receiving notifications before commenting 'I have this issue too'. We welcome additional information that will help resolve this issue. Please read the relevant sections below before commenting.

If you are the original author of the issue

  • If this is resolved, please consider closing it so that the maintainers know not to focus on this.
  • If this might still be an issue, but you are not interested in promoting its resolution, please consider closing it while encouraging others to take over and reopen an issue if they care enough.
  • If you know how to solve the issue, please consider submitting a Pull Request that addresses this issue.
If you are not the original author of the issue

  • If you are also experiencing this issue, please add details of your situation to help with the debugging process.
  • If you know how to solve the issue, please consider submitting a Pull Request that addresses this issue.
Memorandum on closing issues

Don't be afraid to manually close an issue, even if it holds valuable information. Closed issues stay in the system for people to search, read, cross-reference, or even reopen – nothing is lost! Closing obsolete issues is an important way to help maintainers focus their time and effort.

stale[bot] avatar Jul 02 '22 16:07 stale[bot]

I am not sure if TOML name-value pairs are meant to convey an order according to the specification, but (if it is not bad practice to depend on this order) supporting the conversion from a DAG to a TOML file would probably be helpful in cases like these. Otherwise, do you think it would better to close this issue and go with the extra string configuration option?

rcerc avatar Jul 03 '22 16:07 rcerc

The string options are not great - starship isn't the only piece of software that expects a certain ordering, so having proper support for that solves a whole bunch of issues.

peterhoeg avatar Aug 15 '22 07:08 peterhoeg

Thank you for your contribution! I marked this issue as stale due to inactivity. Please be considerate of people watching this issue and receiving notifications before commenting 'I have this issue too'. We welcome additional information that will help resolve this issue. Please read the relevant sections below before commenting.

If you are the original author of the issue

  • If this is resolved, please consider closing it so that the maintainers know not to focus on this.
  • If this might still be an issue, but you are not interested in promoting its resolution, please consider closing it while encouraging others to take over and reopen an issue if they care enough.
  • If you know how to solve the issue, please consider submitting a Pull Request that addresses this issue.
If you are not the original author of the issue

  • If you are also experiencing this issue, please add details of your situation to help with the debugging process.
  • If you know how to solve the issue, please consider submitting a Pull Request that addresses this issue.
Memorandum on closing issues

Don't be afraid to manually close an issue, even if it holds valuable information. Closed issues stay in the system for people to search, read, cross-reference, or even reopen – nothing is lost! Closing obsolete issues is an important way to help maintainers focus their time and effort.

stale[bot] avatar Nov 13 '22 10:11 stale[bot]

https://github.com/nix-community/home-manager/issues/2519#issuecomment-1086876577 is still a very intriguing solution, meaniebot

arcnmx avatar Nov 13 '22 14:11 arcnmx

I'm currently working on tidying (and repairing) the code I suggested to extend Nixpkgs in Home Manager so that we don't eventually run into problems with it, and I'm trying to not break compatibility (too badly, if at all necessary). I can file a draft pull request once I get somewhere though.

rcerc avatar Nov 18 '22 02:11 rcerc

Is #3481 a feasible change?

rcerc avatar Mar 08 '23 14:03 rcerc

this seems like it's going to be a substantially bigger problem in the future: per https://github.com/greshake/i3status-rust/blob/master/NEWS.md#i3status-rust-0307, "In the future block = "..." will be required to be the first field of block configs", i.e. my current configuration (with things like alert = ... first rather than block = ...) would be invalid.

sersorrel avatar Apr 12 '23 12:04 sersorrel

From my understanding, TOML by itself demands that the maps are unordered.

This is not an issue with HM, this is an issue with whatever expects the keys to be ordered.

NobbZ avatar Sep 25 '23 09:09 NobbZ

From my understanding, TOML by itself demands that the maps are unordered.

This is not an issue with HM, this is an issue with whatever expects the keys to be ordered.

You're actually right! (TIL) — https://toml.io/en/v1.0.0#table

Under that, and until the next header or EOF, are the key/values of that table. Key/value pairs within tables are not guaranteed to be in any specific order.

I wonder how feasible it is to change how it works from starship's side though EDIT: Just made an issue upstream: starship/starship#5572

pluiedev avatar Nov 14 '23 07:11 pluiedev