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

nixos: types.str -> types.path #65

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

winterqt
Copy link
Member

No description provided.

@winterqt
Copy link
Member Author

Hm, not sure if this is a good idea after thinking about it further. I'll leave it open just in case someone thinks differently.

@adisbladis
Copy link
Member

I think path's are more "dangerous" as they are easier to mishandle than a string, where the mishandled case leads to copying the files to the store.

@winterqt
Copy link
Member Author

@adisbladis Yeah, that was my reasoning for reconsidering this change. Strings are much harder to misuse in this case.

The main reason this change was proposed in the first place was that it provided a way to check for valid path syntax. How would you feel about changing this to add a check to the string types to check for things such as not being empty, requiring a leading slash, and not having a trailing slash?

@adisbladis
Copy link
Member

How would you feel about changing this to add a check to the string types to check for things such as not being empty, requiring a leading slash, and not having a trailing slash?

That sounds very welcome to me!

@talyz
Copy link
Collaborator

talyz commented Jan 19, 2022

This would be a bit more complicated with #70 in place, where paths in users shouldn't start with a slash, so user paths would have to have a different type/check. Definitely doable, though.

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.

3 participants