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

dock: allow setting spacer tiles #1187

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

khaneliman
Copy link
Contributor

@khaneliman khaneliman commented Nov 21, 2024

You can create spacer tiles in the dock by passing empty tile-data with specific tile-types.

Used to do this in my old dotfiles and have been meaning to upstream a change to support doing this in nix-darwin.
image

@khaneliman khaneliman force-pushed the spacer branch 2 times, most recently from 2915c54 to 9c4bf62 Compare November 21, 2024 21:32
khaneliman added a commit to khaneliman/khanelinix that referenced this pull request Nov 21, 2024
@khaneliman
Copy link
Contributor Author

Sorry, forgot to update test fixture.

@khaneliman khaneliman force-pushed the spacer branch 2 times, most recently from 9a98401 to a9b0e9c Compare November 23, 2024 01:26
@khaneliman khaneliman force-pushed the spacer branch 4 times, most recently from 8ce526e to e05bd61 Compare December 2, 2024 02:39
You can create spacer tiles in the dock by passing empty tile-data with
specific tile-types
@khaneliman
Copy link
Contributor Author

Took me too long to realize that I didn't update my flake's usage for spacer tiles.... I was so confused.

@khaneliman
Copy link
Contributor Author

@Enzime Alright, sorry I forgot about this for a bit. Updated with last suggested changes.

modules/system/defaults/dock.nix Outdated Show resolved Hide resolved
modules/system/defaults/dock.nix Outdated Show resolved Hide resolved
modules/system/defaults/dock.nix Outdated Show resolved Hide resolved
};
};
in
types.nullOr (types.listOf (types.coercedTo (types.either types.str types.path)(path: {app = {inherit path;};}) taggedType));

Choose a reason for hiding this comment

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

I love the type, but I hate the formatting 🙈

Maybe having the other coercedTo args also be defined in the let-in block would help?

Suggested change
types.nullOr (types.listOf (types.coercedTo (types.either types.str types.path)(path: {app = {inherit path;};}) taggedType));
types.nullOr (types.listOf (types.coercedTo simpleType toTagged taggedType));

Comment on lines +158 to +159
type = types.submodule {
options.path = mkOption {
description = "Path to the folder.";
type = types.str;
};
};

Choose a reason for hiding this comment

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

Don't hold me to this, but assuming an app/folder will only ever need a path value, and it's unlikely this submodule will ever expand, I think we could ditch the submodule and just assign a path directly?

folder = mkOption {
  description = "A folder to be added to the dock.";
  type = types.str;
  # (No default)
};
example = [
  { app = "/Applications/Safari.app"; }
  { spacer.small = false; }
  { spacer = { }; }
  { folder = "/System/Applications/Utilities"; }
]

Copy link
Contributor Author

@khaneliman khaneliman Dec 18, 2024

Choose a reason for hiding this comment

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

Yeah, I like this more... I was just trying to fulfill the requested form above. #1187 (comment) @Enzime opinion on simplifying it?

Choose a reason for hiding this comment

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

I think a submodule makes sense if we expect there could be other keys to be set in future, e.g. if it was likely you may set path and name or icon or whatever.

I'm not familiar with the tile-data schema so I don't know how likely that is.

apply =
let
toTile = item: if item ? app then {
tile-data = { file-data = { _CFURLString = item.app.path; _CFURLStringType = 0; }; };

Choose a reason for hiding this comment

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

Maybe I've missed some nix-darwin style preference, but IMO this is more readable as:

tile-data.file-data = {
  _CFURLString = item.app.path;
  _CFURLStringType = 0;
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't seen any formatter configuration or documentation on formatting in this repo. So, I have no idea whether I should just format the whole file with nixfmt or just try to fit it into whats in here...

Choose a reason for hiding this comment

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

There was a discussion in #1054 that would be worth revisiting. I agree that a default format would be really helpful.

modules/system/defaults/dock.nix Outdated Show resolved Hide resolved
Allows passing in a list of attribute sets for different kinds of tiles.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants