symlinked dir support in action_view path

1 view
Skip to first unread message

Rodrigo Rosenfeld Rosas

unread,
Nov 2, 2008, 6:36:56 PM11/2/08
to Ruby on Rails: Core
Some time ago, I posted a doubt about the difference between (found in
action_view/paths.rb)

(Dir.glob("#{@path}/**/*/**") | Dir.glob("#{@path}/**"))

and

Dir.glob("#{@path}/**/*")

No one answered me here neither in ruby-talk list. I've found the
differences a few days when trying to submit a patch. I'm new to git
and I thought that the test errors already existed before my patch.
After I submit, I got a response one test has failed. Then I finally
understood what was the difference.

Ruby doesn't follow symlinks in Dir['**/*'] to avoid infinite loop,
although it is not documented in the official Ruby documentation.

Actually

(Dir.glob("#{@path}/**/*/**") | Dir.glob("#{@path}/**"))

is the same as

(Dir.glob("#{@path}/**/*/**") | Dir.glob("#{@path}/*")) # removed the
last *

This support one level of symlinked directory.

This feature is not documented anywhere and someone could be surprised
her symlinked dir is not being searched by Rails.

All this begin when I was trying to understand the Rails action view
source code. So I think it should be clearer. I could send a patch
removing the extra "*" and adding a comment to explain what that does
so that another developer could avoid this headache.

But then, I realized that I don't really understand what is the
advantage of supporting one level of symlinked directory and why there
is a test to this. Could someone tell me why ActionView needs to
support one-level symlinked directories? And don't you think that
ActionView path documentation should state this limitation with
symlinked directories?

I'm willing to help on this, but I need to think what the core
developers think about this.

Rodrigo.

Michael Koziarski

unread,
Nov 6, 2008, 3:14:48 PM11/6/08
to rubyonra...@googlegroups.com
> I'm willing to help on this, but I need to think what the core
> developers think about this.

I believe the support for a single symlinked directory is probably
because of the way most capistrano deployments work, but you'd have to
dig through the blame output to know for sure. Did that show anything
of interest?

Feel free to add a comment-patch, then we can look at changing the
implementation after 2.2 final ships.


--
Cheers

Koz

Rodrigo Rosenfeld Rosas

unread,
Nov 7, 2008, 7:01:31 AM11/7/08
to Ruby on Rails: Core
Trying to send it again, because I think there was a problem with this
gmail group and Firefox 3. If you've got repeated messages, please
disconsider.

On 6 nov, 17:14, "Michael Koziarski" <mich...@koziarski.com> wrote:
> > I'm willing to help on this, but I need to think what the core
> > developers think about this.
>
> I believe the support for a single symlinked directory is probably
> because of the way most capistrano deployments work, but you'd have to
> dig through the blame output to know for sure. Did that show anything
> of interest?

"you'd have to dig through the blame output to know for sure". Sorry,
I know all the words individual meaning, but I couldn't understand
very well this phrase. English is not my native language. What did you
mean by digging through the blame output?

> Feel free to add a comment-patch, then we can look at changing the
> implementation after 2.2 final ships.

I'm discussing this with Joshua Peek on
http://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/1296-simplify-glob-in-pathsrb-actionpack-to-get-a-cleaner-code#ticket-1296-6

I'm waiting for some orientation of how should I submit this patch...
If I should deprecate this support or not. How should I deprecate in
this case. I need to know if I could comment the code for better
understanding and if I could add a note to Rails documentation about
symlinked dir lack of support or one level support.

My main concern is about making the code clearer. I've spent a lot of
time finding out why that line of code was written that way, when
trying to understand the Rails behaviour for rendering a template...
If at least there was a note in the Ruby documentation for the glob
method stating its symlinked dir limitation, it would be easier to
understand.

Actually, I'm willing to send another patch to Rails guide to explain
how subtemplates can be achieved with Rails. It is not documented in
Rails documentation at all and I found it hard to get some references
at the time, mostly because of a lack of documentation of the render
behaviour. So, I intend to explain this on Rails guides or in the core
documentation. I still don't know which place would be the more
appropriate. While trying to understand why I could do subtemplating
the way I was doing I needed to search the Rails code, since that
method was not well documented. Then I found that line of code that I
couldn't understand why it was so complicated... Even the extra
unnecessary '*' adds further complication...

Rodrigo Rosenfeld Rosas

unread,
Nov 7, 2008, 6:58:44 AM11/7/08
to Ruby on Rails: Core
On 6 nov, 17:14, "Michael Koziarski" <mich...@koziarski.com> wrote:
> > I'm willing to help on this, but I need to think what the core
> > developers think about this.
>
> I believe the support for a single symlinked directory is probably
> because of the way most capistrano deployments work, but you'd have to
> dig through the blame output to know for sure.  Did that show anything
> of interest?

"you'd have to dig through the blame output to know for sure". Sorry,
I know all the words individual meaning, but I couldn't understand
very well this phrase. English is not my native language. What did you
mean by digging through the blame output?

> Feel free to add a comment-patch, then we can look at changing the
> implementation after 2.2 final ships.

Mike Gunderloy

unread,
Nov 7, 2008, 7:27:56 AM11/7/08
to rubyonra...@googlegroups.com
It sounds like that might be appropriate material for the Layouts &
Rendering guide. You can add comments or a patch to the Lighthouse
ticket at http://rails.lighthouseapp.com/projects/16213/tickets/15-layouts-rendering
(don't mind that it's marked "resolved"; we're still happy to take
suggestions for the published guides).

Frederick Cheung

unread,
Nov 7, 2008, 7:44:56 AM11/7/08
to rubyonra...@googlegroups.com

On 7 Nov 2008, at 11:58, Rodrigo Rosenfeld Rosas wrote:

>
> On 6 nov, 17:14, "Michael Koziarski" <mich...@koziarski.com> wrote:
>>> I'm willing to help on this, but I need to think what the core
>>> developers think about this.
>>
>> I believe the support for a single symlinked directory is probably
>> because of the way most capistrano deployments work, but you'd have
>> to
>> dig through the blame output to know for sure. Did that show
>> anything
>> of interest?
>
> "you'd have to dig through the blame output to know for sure". Sorry,
> I know all the words individual meaning, but I couldn't understand
> very well this phrase. English is not my native language. What did you
> mean by digging through the blame output?

the output of git blame (tells you when the lines in a file were last
changed, so you can see who last changed, when and in what commit (and
then hopefully the commit messages tells you why)

Fred

Rodrigo Rosenfeld Rosas

unread,
Nov 7, 2008, 11:16:54 AM11/7/08
to Ruby on Rails: Core
> > "you'd have to dig through the blame output to know for sure". Sorry,
> > I know all the words individual meaning, but I couldn't understand
> > very well this phrase. English is not my native language. What did you
> > mean by digging through the blame output?
>
> the output of git blame (tells you when the lines in a file were last  
> changed, so you can see who last changed, when and in what commit (and  
> then hopefully the commit messages tells you why)

Thanks, know I understand what is the meaning of "blame output". I'm
new to git and I'm not familiar with all commands yet.

I used it to find out that the line is there since the file was
created, so it is not that useful in this case :) Maybe there is
another git command to search for the line in another file in the
history before template preloading was refactored...

Rodrigo.

Rodrigo Rosenfeld Rosas

unread,
Nov 7, 2008, 11:41:16 AM11/7/08
to Ruby on Rails: Core
On 7 nov, 10:27, Mike Gunderloy <larkw...@gmail.com> wrote:
> It sounds like that might be appropriate material for the Layouts &  
> Rendering guide. You can add comments or a patch to the Lighthouse  
> ticket athttp://rails.lighthouseapp.com/projects/16213/tickets/15-layouts-rend...
>   (don't mind that it's marked "resolved"; we're still happy to take  
> suggestions for the published guides).

Ok, I'll send my patches to that ticket when I write it. It won't
happen before tuesday, since I'm traveling tonight...

Rodrigo.
Reply all
Reply to author
Forward
0 new messages