-
-
Notifications
You must be signed in to change notification settings - Fork 73
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
Make os.Path and os.RelPath implement CharSequence #247
Conversation
I can't spot a difference. I don't think |
Having os.Path and os.RelPath, in fact os.FilePath, implement CharSequence reduces the boilerplate when using functions that take a input collection and produce some form of textual output. Consider the following example def printBashArray(out: Appendable, name: String, content: Seq[CharSequence]) = out.append(f"${name}=(\n") for c <- content do out.append(f"\t${c}\n") out.append(")\n") and val fooPaths: Seq[os.RelPath] Currently we have to printBashArray(out, "FOO_PATHS", fooPaths.map(_.toString)) after this change, we can simply printBashArray(out, "FOO_PATHS", fooPaths) (Of course, we could also declare 'content' as Seq[Any], but this blurres the intention.)
383bff3
to
103cbe7
Compare
Sorry, copy&paste mistake. Now fixed.
This PR is about I have made good experience with making specialized types1 that are essentially a sequence of characters (minus potentially additionally restrictions) implement the 1: Examples include |
I'm still not convinced. I even don't understand your motivation at all. If you need a printBashArray(out, "FOO_PATHS", fooPaths.toString) A path is not a char sequence. E.g. |
I think you may have missed that |
If feeding implicit def pathToCharSeq(path: os.Path): CharSequence = path.toString |
Yes, I do it constantly. Again, if you have a dedicated type for something that is (typically) expressed as a String, like
Unfortunately, this will not help. This is not a case where implicit or given Conversation will be applied. As soon as the to-be-converted type is the parameterized type of a generic class, as is the case here, the implicit conversation to the expected type will not be applied. For example: #!/usr/bin/env -S scala-cli shebang
//> using scala "3.3.1"
//> using jvm "system"
//> using dep "com.lihaoyi::os-lib:0.9.3"
object GivenConversion:
given Conversion[os.FilePath, CharSequence] = (f: os.FilePath) => f.toString()
def myMkString(input: Seq[CharSequence]) = input.map(_.toString).mkString("- ", "\n- ", "\n")
val paths = Seq(
os.Path("/foo"),
os.Path("/bar"),
)
import GivenConversion.*
myMkString(paths) Results in
I wonder where this aversion against subclassing Do you want os-lib users to deal with additional friction when using the os-lib API although it could be avoided? |
fwiw, I agree with @lefou — tons of data types have a string representation that is relatively frequently needed, but that doesn't mean they should all implement |
Note that the Java standard library designers seem to agree as well:
|
Trying to help you with your use case, I think your example should work with implicit def pathToCharSeq(paths: Seq[os.Path]): Seq[CharSequence] = paths.map(_.toString)
Wow. "Aversion". I think you misunderstood something. And I don't like the tone.
I see it as @SethTisue. A domain type using a string internally, isn't necessarily a string itself. Also, I wrote before:
Doesn't that qualify as at least two downsides?
Absolutely not. I simply don't agree this is an issue that produces "friction". |
I am sorry, that was not intended to be in any way an attack. Maybe my understanding what 'aversion' expresses is not correct.
That is something I do not understand: Assume
I like the Java standard library very much. I think it is one of the best designed standard libraries out there. That said, it has its warts and this is one of those. I leave at it that and with saying again that I am sorry if my reply came across as impolite. That wasn't my intention, I just wanted to learn about the arguments against this change. |
@Flowdalic, since you asked, let me add a last comment to this PR.
The latter case is just some known string API on a string, and we all know strings are arrays or sequences of characters. Since its concept is that old, it's rather "intuitive" API. But in the former case, we're dealing with a path. So what could So, since a I understand the desire to automagically support your favorite domain types everywhere. But implementing a To pick up your other example, the XML writer; in Scala, the preferred or typical way to support different domain type to be written as XML is the concept of type classes. |
Thanks for the example. That provides some insight about why you reject this change. However the rationale
i.e., that the behavior of The If you return anything else than the string from the 3 to 7th (exclusive) char, the you would violate the contract of There is really no ambiguity introduced by this PR about what |
Having os.Path and os.RelPath, in fact os.FilePath, implement CharSequence reduces the boilerplate when using functions that take a input collection and produce some form of textual output.
Consider the following example
and
Currently we have to
printBashArray(out, "FOO_PATHS", fooPaths.map(_.toString))
after this change, we can simply
printBashArray(out, "FOO_PATHS", fooPaths)
(Of course, we could also declare 'content' as Seq[Any], but this blurres the intention.)