Skip to content

Conversation

@BurdetteLamar
Copy link
Member

No description provided.

@BurdetteLamar BurdetteLamar requested a review from nobu January 2, 2026 00:43
@BurdetteLamar BurdetteLamar added the documentation Improvements or additions to documentation label Jan 2, 2026
Comment on lines 656 to 657
# When +other+ specifies a relative path (see #relative?),
# equivalent to <tt>Pathname.new(self.to_s + other.to_s)</tt>:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not the case, e.g.:

irb(main):002> Pathname("/a/b") + Pathname("../c")
=> #<Pathname:/a/c>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow! Thanks, @eregon! I had no idea (from the existing doc) of this handling. I should have consulted the tests.

Comment on lines +650 to +651
# call-seq:
# self + other -> new_pathname
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any advantage to have a call-seq for a method defined in Ruby?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so.

  • The "self + other" idiom makes clear the "binary-operator" nature of the metho; it is seen in many other places in Ruby documentation.
  • The "-> new_pathname" is useful information.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's all redundant for + which is always binary and always returns a new value (doesn't mutate the receiver).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Always binary in Pathname, but other classes have @+, so the distinction is not fatuous.

No one (at least no one I know) writes pn.+('baz'), so self + other will seem natural to most?

new_pathname is indeed redundant, but is the widespread custom in the Ruby core documentation.

@eregon
Copy link
Member

eregon commented Jan 2, 2026

What's the issue with the existing docs? IMO they are already fine, and concise is IMO good as long as still clear.

@BurdetteLamar BurdetteLamar requested a review from eregon January 3, 2026 16:51
Comment on lines +661 to +703
# Extra component separators (<tt>'/'</tt>) are removed:
#
# Pathname.new('/a/b/') + 'c' # => #<Pathname:/a/b/c>
#
# Extra current-directory components (<tt>'.'</tt>) are removed:
#
# Pathname.new('a') + '.' # => #<Pathname:a>
# Pathname.new('.') + 'a' # => #<Pathname:a>
# Pathname.new('.') + '.' # => #<Pathname:.>
#
# Parent-directory components (<tt>'..'</tt>) are:
#
# - Resolved, when possible:
#
# Pathname.new('a') + '..' # => #<Pathname:.>
# Pathname.new('a/b') + '..' # => #<Pathname:a>
# Pathname.new('/') + '../a' # => #<Pathname:/a>
# Pathname.new('a') + '../b' # => #<Pathname:b>
# Pathname.new('a/b') + '../c' # => #<Pathname:a/c>
# Pathname.new('a//b/c') + '../d//e' # => #<Pathname:a//b/d//e>
#
# - Removed, when not needed:
#
# Pathname.new('/') + '..' # => #<Pathname:/>
#
# - Retained, when needed:
#
# Pathname.new('..') + '..' # => #<Pathname:../..>
# Pathname.new('..') + '../a' # => #<Pathname:../../a>
#
# When +other+ specifies an absolute path (see #absolute?),
# equivalent to <tt>Pathname.new(other.to_s)</tt>:
#
# Pathname.new('/a') + '/b/c' # => #<Pathname:/b/c>
#
# Occurrences of <tt>'/'</tt>, <tt>'.'</tt>, and <tt>'..'</tt> are preserved:
#
# Pathname.new('/a') + '//b//c/./../d' # => #<Pathname://b//c/./../d>
#
# This method does not access the file system, so +other+ need not represent
# an existing (or even a valid) file or directory path:
#
# Pathname.new('/var') + 'nosuch:ever' # => #<Pathname:/var/nosuch:ever>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks potentially useful information, OTOH it also describes a lot of things, some of which might be implementation details.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see implementation details here. These behaviors are user-visible, and are mostly taken directly from the tests.

Copy link
Member

@eregon eregon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@eregon eregon merged commit e0452b4 into ruby:master Jan 4, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants