search

com-lihaoyi / os-lib

OS-Lib is a simple, flexible, high-performance Scala interface to common OS filesystem and subprocess APIs
Other
690 stars 71 forks source link

make os.root.baseName equal to s"${os.root/}" rather than throwing an exception #277

Closed philwalk closed 3 months ago

philwalk commented 4 months ago

This addresses the problems described by #276 It provides a fix for https://github.com/VirtusLab/scala-cli/issues/2954 It makes os.root.last the empty string rather than throwing LastOnEmptyPath. Then, it defines os.root.baseName as s"$driveRoot/", rather than throwing an Exception.

lihaoyi commented 3 months ago

TBH I'm not sure if this is the correct behavior for this. I think it's reasonable for it to fail and downstream to handle it, v.s. arbitrarily returning empty strings.

There are tons of cases where empty strings are not what you want either, and they have a tendency to propagate far beyond the point of creation before misbehaving in complicated ways. In comparison, the exception immediately tells you what's going on and you can remediate it at the failure site

philwalk commented 3 months ago

TBH I'm not sure if this is the correct behavior for this. I think it's reasonable for it to fail and downstream to handle it, v.s. arbitrarily returning empty strings.

I don't disagree with your reasoning. A concern is that throwing of an exception makes it unsafe to call .baseName on any os.Path that might be === os.root. Empty string is a reasonable way to represent that a path has no segments, implying that it's os.root. This is similar to the way that dirname / returns / rather than an error message.

On the other hand, this change might cause problems with existing client code, so perhaps this needs to be handled the way you suggest, thanks for the feedback.