https://bugs.ruby-lang.org/https://bugs.ruby-lang.org/favicon.ico?17113305112014-07-10T08:08:03ZRuby Issue Tracking SystemRuby master - Bug #10011: Passing a string to Pathname#relative_path_from results in NoMethodErrorhttps://bugs.ruby-lang.org/issues/10011?journal_id=476762014-07-10T08:08:03Zleriksen (Leif Eriksen)leif.eriksen.au@gmail.com
<ul></ul><p>ok so I have 2 possible fixes, one of which I've implemented</p>
<p>The unimplemented fix is maybe a new method along the lines of to_s, but for paths, called to_pathname. Then we could reopen String and define<br>
clas String<br>
def to_pathname<br>
Pathname.new self<br>
end<br>
end</p>
<p>And so forth for any classes where converting to a pathname makes some semantic sense.</p>
<p>Then in relative_path_from can just do</p>
<p>base_directory.to_pathname.cleanpath.to_s at line 493, as in the above description.</p>
<p>But I can see that might be harder to get through than the actual, fairly naive fix I've done, which is to just convert base_directory to a Pathname if required,as per<br>
[[https://github.com/leriksen/ruby/commit/0ab29978bb83e485784fa426340ce91df30d2d13<br>
]]</p>
<p>The test code seems to imply this is probably quite acceptable.</p>
<p>If the commit is preferred, I'll do a pull request tomorrow unless there are issues expressed here.</p>
<p>Or if the :to_pathname option is preferred, I implement that instead.</p> Ruby master - Bug #10011: Passing a string to Pathname#relative_path_from results in NoMethodErrorhttps://bugs.ruby-lang.org/issues/10011?journal_id=476862014-07-10T15:25:09ZAnonymous
<ul></ul><p>I don't think we should add a method to String; there is an existing path coercion protocol (<code>to_path</code> and <code>File.path</code>) and even though String does not implement <code>to_path</code>, I think having a <code>to_pathname</code> would be confusing.</p>
<p>Anyway, consider concatenation:</p>
<pre><code class="ruby syntaxhl" data-language="ruby"><span class="nb">require</span> <span class="s2">"pathname"</span>
<span class="n">a</span> <span class="o">=</span> <span class="no">Pathname</span><span class="p">.</span><span class="nf">new</span><span class="p">(</span><span class="s2">"a"</span><span class="p">)</span> <span class="c1"># => #<Pathname:a></span>
<span class="n">a</span> <span class="o">+</span> <span class="s2">"b"</span> <span class="c1"># => #<Pathname:a/b></span>
<span class="n">a</span> <span class="o">+</span> <span class="no">Object</span><span class="p">.</span><span class="nf">new</span> <span class="c1"># => TypeError: no implicit conversion of Object into String</span>
</code></pre>
<p>Mirroring this behavior in <code>relative_path_from</code> can be accomplished with a simpler patch:</p>
<pre><code class="diff syntaxhl" data-language="diff"><span class="gh">diff --git a/ext/pathname/lib/pathname.rb b/ext/pathname/lib/pathname.rb
index e61aa2c..e613afd 100644
</span><span class="gd">--- a/ext/pathname/lib/pathname.rb
</span><span class="gi">+++ b/ext/pathname/lib/pathname.rb
</span><span class="p">@@ -490,7 +490,7 @@</span> def each_child(with_directory=true, &b)
#
def relative_path_from(base_directory)
dest_directory = self.cleanpath.to_s
<span class="gd">- base_directory = base_directory.cleanpath.to_s
</span><span class="gi">+ base_directory = Pathname.new(base_directory).cleanpath.to_s
</span> dest_prefix = dest_directory
dest_names = []
while r = chop_basename(dest_prefix)
</code></pre> Ruby master - Bug #10011: Passing a string to Pathname#relative_path_from results in NoMethodErrorhttps://bugs.ruby-lang.org/issues/10011?journal_id=476912014-07-10T23:19:41Zleriksen (Leif Eriksen)leif.eriksen.au@gmail.com
<ul></ul><p>I agree with the confusion - initially I thought :to_path could be used, but despite its name it returns a string, hence the suggestion for :to_pathname, that allows things to implement a way to have themselves converted to a Pathname.</p>
<p>I was going to do it your way initially, but I decided on the slightly longer version because</p>
<ol>
<li>I didn't want to call Pathname::new if we already had been passed a Pathname (or class that inherits from that) - I think preserving the original subclass is important</li>
<li>I wanted an nice exception, related to trying to create the Pathname, raise right at that point, to explain where the user had gone wrong.</li>
</ol>
<p>What you have in your code is pretty much what <em>all</em> the tests for relative_path_from do, which might have been taken as a sign on the original implementation, however that was > 8 years ago.</p>
<p>Unless you (or someone else) thinks my reasons are poor, I'll commit as is in 24 hours.</p> Ruby master - Bug #10011: Passing a string to Pathname#relative_path_from results in NoMethodErrorhttps://bugs.ruby-lang.org/issues/10011?journal_id=476932014-07-10T23:48:10ZAnonymous
<ul></ul><p>At the end of the day, I just want something more friendly than a <code>NoMethodError</code> from inside the method, so I won't stand in the way of progress, but...</p>
<blockquote>
<ol>
<li>I didn't want to call Pathname::new if we already had been passed a Pathname (or class that inherits from that) - I think preserving the original subclass is important</li>
</ol>
</blockquote>
<p>I'm not sure how important that is in this case. This method only requires a Pathname so that it can call <code>cleanpath</code> on it, after which it is converted into a string. I guess someone may have overridden <code>cleanpath</code> in a subclass, but since calling it is just an implementation detail of <code>relative_path_from</code> I would argue that there is no expectation it will be called. Or am I missing a different reason?</p>
<blockquote>
<ol start="2">
<li>I wanted an nice exception, related to trying to create the Pathname, raise right at that point, to explain where the user had gone wrong.</li>
</ol>
</blockquote>
<p>I see your point, though I do think TypeError is more appropriate. And it would be consistent with, for example:</p>
<pre><code class="ruby syntaxhl" data-language="ruby"><span class="no">File</span><span class="p">.</span><span class="nf">rename</span> <span class="s2">"a"</span><span class="p">,</span> <span class="no">Object</span><span class="p">.</span><span class="nf">new</span> <span class="c1"># => TypeError: no implicit conversion of Object into String</span>
</code></pre>
<p>It seems ArgumentError is usually raised when there is something "wrong" with the argument (there are two examples in the body of <code>relative_path_from</code>) and TypeError when the incorrect type is passed.</p> Ruby master - Bug #10011: Passing a string to Pathname#relative_path_from results in NoMethodErrorhttps://bugs.ruby-lang.org/issues/10011?journal_id=476952014-07-11T00:41:45Zleriksen (Leif Eriksen)leif.eriksen.au@gmail.com
<ul></ul><p>OK I will update to TypeError - the method argument does need to support the Pathname interface.</p>
<p>As for the subclass - yes that is mostly it. It's a small detail.</p>
<p>Thanx for the feedback.</p> Ruby master - Bug #10011: Passing a string to Pathname#relative_path_from results in NoMethodErrorhttps://bugs.ruby-lang.org/issues/10011?journal_id=477392014-07-13T10:38:09Zleriksen (Leif Eriksen)leif.eriksen.au@gmail.com
<ul></ul><p><a href="https://github.com/ruby/ruby/pull/666" class="external">https://github.com/ruby/ruby/pull/666</a></p> Ruby master - Bug #10011: Passing a string to Pathname#relative_path_from results in NoMethodErrorhttps://bugs.ruby-lang.org/issues/10011?journal_id=480332014-07-26T02:08:05Znaruse (Yui NARUSE)naruse@airemix.jp
<ul><li><strong>Status</strong> changed from <i>Open</i> to <i>Assigned</i></li><li><strong>Assignee</strong> set to <i>akr (Akira Tanaka)</i></li></ul> Ruby master - Bug #10011: Passing a string to Pathname#relative_path_from results in NoMethodErrorhttps://bugs.ruby-lang.org/issues/10011?journal_id=480642014-07-26T15:50:36Zzzak (zzak _)
<ul><li><strong>Category</strong> set to <i>ext</i></li><li><strong>Target version</strong> set to <i>2.2.0</i></li></ul> Ruby master - Bug #10011: Passing a string to Pathname#relative_path_from results in NoMethodErrorhttps://bugs.ruby-lang.org/issues/10011?journal_id=693302018-01-05T21:01:16Znaruse (Yui NARUSE)naruse@airemix.jp
<ul><li><strong>Target version</strong> deleted (<del><i>2.2.0</i></del>)</li></ul> Ruby master - Bug #10011: Passing a string to Pathname#relative_path_from results in NoMethodErrorhttps://bugs.ruby-lang.org/issues/10011?journal_id=805652019-08-10T15:18:50Zjeremyevans0 (Jeremy Evans)merch-redmine@jeremyevans.net
<ul><li><strong>Status</strong> changed from <i>Assigned</i> to <i>Closed</i></li></ul><p>This was fixed between Ruby 2.5 and 2.6:</p>
<pre><code>$ ruby25 -r pathname -e 'p Pathname.new("/usr/bin/cc").relative_path_from("/usr/bin")'
Traceback (most recent call last):
1: from -e:1:in `<main>'
/usr/local/lib/ruby/2.5/pathname.rb:508:in `relative_path_from': undefined method `cleanpath' for "/usr/bin":String (NoMethodError)
$ ruby26 -r pathname -e 'p Pathname.new("/usr/bin/cc").relative_path_from("/usr/bin")'
#<Pathname:cc>
</code></pre>