Clarify documentation on Path#join and #==

[straight-shoota]

Ref. https://forum.crystal-lang.org/t/path-join/2988/6

[Leximatica]

Regarding "the second to last component is returned."

What about the patholexical edge case:

Path["/foo/bar/././././."].basename

If "bar" is the second-to-last component of Path["/foo/bar/."], then it is the sixth-to-last component of Path["/foo/bar/././././."], and thus the documentation is rendered literally wrong.

I wonder if we are converging on an understanding that Path[p].basename should be an alias for Path[p].normalize.basename.

This suggests a possible:

Path#basename(normalize : Boolean = true)

... which "does no harm", and provides a capability to handle (i.e. decompose and explore) strangely formed paths within Path, without having to export "to_s" and write custom (potentially platform dependent) parsers.

[straight-shoota]

The first part of the sentence clearly applies it to the "case the last component is the empty string". . is not an empty string. So Path["a/."].basename == "." and the documentation is correct.

I wonder if we are converging on an understanding that Path[p].basename should be an alias for Path[p].normalize.basename.

I don't think #basename nor any other method should implicitly normalize the path unless necessary (for example in #relative_to?). Not even if you can opt-out of normalization. You can simply opt-in by a calling #normalize. Ideally as soon as a potentially de-normalized path enters the system, which means only once instead of every time you call a method #basename.

... which "does no harm", and provides a capability to handle (i.e. decompose and explore) strangely formed paths within Path, without having to export "to_s" and write custom (potentially platform dependent) parsers.

"strangely formed paths" are already handled by #normalize. There's no need for to_s or custom parsers. Just a single method.

[Leximatica]

With all due respect, my comment was in respect to the proposed changes to the documentation, which specifies the case: Path["/foo/bar/."].basename # => "bar" Did you overlook that?

…

On Sun, 28 Feb 2021 at 12:36, Johannes Müller ***@***.***> wrote: The first part of the sentence clearly applies it to the "case the last component is the *empty string*". . is not an empty string. So Path["a/."].basename == "." and the documentation is correct. I wonder if we are converging on an understanding that Path[p].basename should be an alias for Path[p].normalize.basename. I don't think #basename nor any other method should implicitly normalize the path unless necessary (for example in #relative_to?). Not even if you can opt-out of normalization. You can simply opt-in by a calling #normalize. Ideally as soon as a potentially de-normalized path enters the system, which means only once instead of every time you call a method #basename. ... which "does no harm", and provides a capability to handle (i.e. decompose and explore) strangely formed paths within Path, without having to export "to_s" and write custom (potentially platform dependent) parsers. "strangely formed paths" are already handled by #normalize. There's no need for to_s or custom parsers. Just a single method. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#10455 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALMVB2EJNCG7LHQUSQ2HXMDTBKSNRANCNFSM4YLCIEMA> .

[straight-shoota]

Ah, sorry I didn't catch what you mean. That's just a typo. It should be Path["/foo/bar/."].basename # => ".".

[oprypin]

Hmm this sample code is a bit misleading because it says == but then only after a big chunk of whitespace it says "actually that's false".

On the other hand, if you change this to !=, then adding # => true at the end is also misleading.

Maybe just have to drop the comments at the end..

[straight-shoota]

I see what you mean, but IMO this is the least surprising variant. Besides, we use this pattern everywhere for documentation. I don't think we should change anything. Using != doesn't help, because the visual difference is even less than with == and false.

[oprypin]

OK that's fine.
Could still try to minimize the whitespace.

Suggested change

	# Path["foo"] == Path["foo"] # => true
	# Path["foo"] == Path["./foo"] # => false
	# Path["foo"] == Path["foo/"] # => false
	# Path.posix("foo") == Path.windows("foo") # => false
	# Path["foo"] == Path["foo"] # => true
	# Path["foo"] == Path["./foo"] # => false
	# Path["foo"] == Path["foo/"] # => false
	#
	# Path.posix("foo") == Path.windows("foo") # => false

[straight-shoota]

I don't know. Is the whitespace really such a visual barrier in the rendered API docs? IMO adding different indents might add confusion, as well as the random empty line, just to force the formatted.
All these expressions form a set. The last one is just visually different because it uses explicitly named constructor methods. But semantically, there should be no distance to the previous ones.

We could consider using explicit .new methods instead of the .[] short form. That would reduce the difference in lenght between the lines.