symlinked dir support in action_view path

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.

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.

> 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...

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.

> 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...

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).

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

> "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 :slight_smile: Maybe there is another git command to search for the line in another file in the history before template preloading was refactored...

Rodrigo.

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.