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

Consider using a string as the path type #2

Open
lpil opened this issue Dec 26, 2022 · 5 comments
Open

Consider using a string as the path type #2

lpil opened this issue Dec 26, 2022 · 5 comments

Comments

@lpil
Copy link

lpil commented Dec 26, 2022

Hello!

Currently this library represents paths as a 2 variant custom type that each hold a list of strings. I think it would be advantageous to instead use a string:

  • It will make this library easier to use with other libraries. If the path type is defined in this library then any other library that uses it will have to depend on this one, however if it uses a core type than we don't need to add this dep to all other path using libraries.
  • It removes boilerplate from the library as there's less wrapping and unwrapping to get to and from the input strings and strings to be given to the path using functions.
  • It makes FFI easier as it's the same type that both targets use.
  • It is conceptually simpler and easier to read.

Thanks,
Louis

@hayleigh-dot-dev
Copy link
Member

A String is not necessarily a valid Path – a huge benefit of static type systems is that we can more precisely model things. Wrapping primitives in an opaque custom type and providing constructors/conversions is a very common practice in other languages like Elm or Haskell and strikes a reasonable balance between safety and usability without requiring a more sophisticated type system.

It also makes the API more flexible to internal changes – right now we use a list of strings but in the future we may use just a single string, for example. That can be introduced as a patch change without affecting the public API in a way that is not possible if we simply expose a type alias.

It makes FFI easier as it's the same type that both targets use.

For those heavily invested in FFI-ing a particular library or API they can simply use that instead of this package. For everyone else, we think it's of greater benefit to have a standard target-agnostic Path representation for "Gleam-first" applications. In situations where users are dealing with FFI code that they know produces valid paths, they can always assert Ok(path) = ... when calling one of the constructors – that's why Gleam has assertions after all!

It is conceptually simpler and easier to read.

I'd actually argue that concealing implementation details can make these things easier to understand by limiting and defining the scope of the type/module. It makes consuming the code easier because the types guarantee that things are valid.

@lpil
Copy link
Author

lpil commented Dec 29, 2022

A String is not necessarily a valid Path

This is true but the abstraction offered by this library does not offer better guarentees of correctness than the String type does. It does not check that the path is correct in the constructor, and the underlying List(String) is not capable of representing all valid paths, and the to_string function specifies that a path can always be turned into a string without error, which is not correct. Futher the constructor forces incorrect handling of paths on non unix-like platforms.

This could be fixed but it would be a large amount of work and I don't believe it will result in a better UX.

assert Ok(path) = path.parse(input_path)
assert Ok(str) = path.join("wibble") |> path.to_string
assert Ok(txt) = file.read(str)

vs

assert Ok(txt) = input_path |> path.join("wibble") |> file.read

An API like this could be more accurate than one that uses a string, but I'm not convinced that wouldn't correctness at the expense of usefulness.

@aslilac
Copy link
Contributor

aslilac commented Dec 29, 2022

and the underlying List(String) is not capable of representing all valid paths

how so? not convinced there's anything a String can represent that a List(String) can't

@lpil
Copy link
Author

lpil commented Dec 29, 2022

Neither String or List(String) can represent all paths because String only supports unicode while there is no such requirement on major operating systems, so this type is not more correct than String.

@aslilac
Copy link
Contributor

aslilac commented Dec 29, 2022

now that's actually a good point, Rust uses OsStr and OsString internally for its Path and PathBuf types because unicode. we should probably use BitString internally (however that looks).

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

No branches or pull requests

3 participants