Add literal syntax for os.Path, os.SubPath, os.RelPath#353
Merged
Conversation
lihaoyi
added a commit
to com-lihaoyi/mill
that referenced
this pull request
Feb 6, 2025
…bpath string literals (#4486) First step in #4447, by providing an alternative to the previous `os.Path` APIs. Effectively this allows us to replace ```scala def mainScript = Task.Source { millSourcePath / "src/foo.py" } ``` with ```scala def mainScript = Task.Source { "src/foo.py" } ``` Pulls in com-lihaoyi/os-lib#353 from upstream to make constructing `os.SubPath`s more ergonomic by eliding the lead `os.sub /` prefix in the case of literal strings while still maintaining a degree of safety: * "outer" paths starting with `..`s and absolute paths starting with `/` are rejected at compile time * Only literal strings are converted implicitly, anything non-literal needs to be an explicit `os.SubPath` For now we provide this as an alternative to passing in an absolute `os.Path`, but probably 99% of scenarios should be using this sub-path API rather than absolute paths since (a) it's more concise and (b) your sources should be within your `millSourcePath` anyway. I'm not sure we can get rid of the `os.Path`-taking API entirely, but we can definitely de-prioritize it and call it `SourcesUnsafe` or something so that anyone who needs it can use it but most people won't use it accidentally
lihaoyi
added a commit
to com-lihaoyi/mill
that referenced
this pull request
Feb 6, 2025
…tly from subpath string literals (#4487) First step in #4447, by providing an alternative to the previous `os.Path` APIs. Effectively this allows us to replace ```scala def mainScript = Task.Source { millSourcePath / "src/foo.py" } ``` with ```scala def mainScript = Task.Source { "src/foo.py" } ``` Pulls in com-lihaoyi/os-lib#353 from upstream to make constructing `os.SubPath`s more ergonomic by eliding the lead `os.sub /` prefix in the case of literal strings while still maintaining a degree of safety: * "outer" paths starting with `..`s and absolute paths starting with `/` are rejected at compile time * Only literal strings are converted implicitly, anything non-literal needs to be an explicit `os.SubPath` For now we provide this as an alternative to passing in an absolute `os.Path`, but probably 99% of scenarios should be using this sub-path API rather than absolute paths since (a) it's more concise and (b) your sources should be within your `millSourcePath` anyway. I'm not sure we can get rid of the `os.Path`-taking API entirely, but we can definitely de-prioritize it and call it `SourcesUnsafe` or something so that anyone who needs it can use it but most people won't use it accidentally
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR allows
This only allows string-literals that are valid absolute/sub/relative-path respectively; passing in invalid paths (e.g.
val p: os.Path = "hello/world") or non-literals (e.g.val str = "/hello/world"; val s: os.SubPath = str) is a compile errorThis builds upon @pawelsadlo's work in #297, mostly using
segmentsFromStringLiteralValidationunchanged with some light pre/post processing to trim the leading/off of absoluteos.Paths and check for leading..s onos.SubPathsI'm going to declare bankruptcy on the Expecty issues, as we cannot forever be working around bugs in unrelated libraries. If someone has problems and wants to fix expecty, they can do so, and we don't need to care. If nobody cares enough to fix expecty, we shouldn't care either.