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

Update FilePath constructors to allow a non-partial way to create a FilePath #3819

Merged
merged 26 commits into from
Sep 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
289f87f
Initial work towards splitting FilePath
rhagenson Aug 4, 2021
4781c41
Type changes in tests; rm partial ? denotations
rhagenson Aug 4, 2021
4f1ce12
Update FilePath in examples
rhagenson Aug 5, 2021
cce778e
Fix additional space
rhagenson Aug 5, 2021
691b958
Add FilePath.from no-error test
rhagenson Aug 10, 2021
3e2374c
Move FilePath.from test with other FilePath tests
rhagenson Aug 10, 2021
7c373fb
Add successful and failing FilePath.from tests
rhagenson Aug 10, 2021
37eaa2d
Add release notes
rhagenson Aug 10, 2021
e102754
Update release notes
rhagenson Aug 19, 2021
3f7a007
Add newline
rhagenson Aug 19, 2021
5c28409
Shorten construction to one line
rhagenson Aug 19, 2021
7fd83be
Add docstring explaining error
rhagenson Aug 19, 2021
33f7700
Informative error message
rhagenson Aug 19, 2021
bf02ca8
Use match to remove try-block
rhagenson Aug 19, 2021
647f97f
One space between unit tests
rhagenson Aug 19, 2021
306e958
? at end of constructor, no newline
rhagenson Aug 19, 2021
651663f
Remove tmp ambient variables, single line construction
rhagenson Aug 19, 2021
7bc7640
FileAuth.create uses union of AmbientAuth and FileAuth
rhagenson Aug 29, 2021
75759bc
Docstring clarity
rhagenson Sep 1, 2021
6581651
Update with AmbientAuth in union
rhagenson Sep 1, 2021
a497c18
Revert use of FileAuth over AmbientAuth
rhagenson Sep 1, 2021
d6bdb27
RM temp variable
rhagenson Sep 2, 2021
01b10cf
Clarify docstring re: capabilities
rhagenson Sep 2, 2021
ae327e8
RM talk of pattern and reformat to style guide
rhagenson Sep 2, 2021
135578d
FileAuth(auth) -> auth
rhagenson Sep 2, 2021
6eb0a20
Clarify before and after descriptions of changes
rhagenson Sep 2, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions .release-notes/split-filepath.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
## Split `FilePath` construction into two methods

`FilePath` previously had only one constructor, the default `create`, which used a union of `(AmbientAuth | FilePath)`. The first half of this union `AmbientAuth` could never `error`, while the second `FilePath` could `error`. By splitting construction into two methods, we now have a default `create` constructor which cannot `error` and a new `from` constructor which can `error`. The default `create` constructor now accepts a new "files" package root authority `FileAuth` as well as the global root authority `AmbientAuth`, while the new `from` constructor uses `FilePath`.

The result of this change is that three user changes are needed, namely around `create`, `from`, and `mkdtemp`. Any place where previously `AmbientAuth` was accepted now also accepts `FileAuth`.

Prior to this change, `create` could be used with `AmbientAuth` as in:

```pony
let ambient: AmbientAuth = ...
let filepath: FilePath = FilePath(ambient, path)?
```

After this change, `create` can be used with `AmbientAuth` or `FileAuth` -- note that this can no longer fail:

```pony
let ambient: AmbientAuth = ...
let filepath: FilePath = FilePath(ambient, path)
```

or

```pony
let fileauth: FileAuth = ...
let filepath: FilePath = FilePath(fileauth, path)
```

---

Prior to this change, `create` could be used with `FilePath` as in:

```pony
let filepath: FilePath = ...
let subpath = FilePath(filepath, path)?
```

After this change, construction with an existing `FilePath` must use `from`:

```pony
let filepath: FilePath = ...
let subpath = FilePath.from(filepath, path)?
```

---

Prior to this change, `mkdtemp` could be used with `AmbientAuth` or `FilePath` as in:

```pony
let ambient: AmbientAuth = ...
let tempdir = FilePath.mkdtemp(ambient, prefix)?
```

or

```pony
let filepath: FilePath = ...
let tempdir = FilePath.mkdtemp(filepath, prefix)?
```

After this change, `mkdtemp` can also use `FileAuth` -- note can still fail:

```pony
let fileauth: FileAuth = ...
let tempdir = FilePath.mkdtemp(fileauth, prefix)?
```
2 changes: 1 addition & 1 deletion examples/files/files.pony
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ actor Main

try
with file = OpenFile(
FilePath(env.root as AmbientAuth, env.args(1)?, caps)?) as File
FilePath(env.root as AmbientAuth, env.args(1)?, caps)) as File
do
env.out.print(file.path.path)
for line in file.lines() do
Expand Down
4 changes: 2 additions & 2 deletions examples/mandelbrot/mandelbrot.pony
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class val Config
let outpath: (FilePath | None)

new val create(env: Env) ? =>
let cs = CommandSpec.leaf("gups_opt",
let cs = CommandSpec.leaf("run",
rhagenson marked this conversation as resolved.
Show resolved Hide resolved
"""
The binary output can be converted to a PNG with the following command
(ImageMagick Tools required):
Expand Down Expand Up @@ -114,7 +114,7 @@ class val Config
width = cmd.option("width").i64().usize()
outpath =
try
FilePath(env.root as AmbientAuth, cmd.option("output").string())?
FilePath(env.root as AmbientAuth, cmd.option("output").string())
else
None
end
Expand Down
Loading