[PATCH/puppet 1/1] Fix #2831 - puppetdoc doesn't cope with regex node

[Brice Figureau]

The problem is that regex node contains '/' which is a directory
separator on unix.
Since puppetdoc writes a file for each node this was creating empty
directories and documentation for such node couldn't be stored.
This patch removes the slashes in the node names.

Signed-off-by: Brice Figureau <brice-...@daysofwonder.com>
---
.../util/rdoc/generators/puppet_generator.rb | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/lib/puppet/util/rdoc/generators/puppet_generator.rb b/lib/puppet/util/rdoc/generators/puppet_generator.rb
index bf2609f..2ceb170 100644
--- a/lib/puppet/util/rdoc/generators/puppet_generator.rb
+++ b/lib/puppet/util/rdoc/generators/puppet_generator.rb
@@ -411,6 +411,7 @@ module Generators
# which is also its url
def http_url(full_name, prefix)
path = full_name.dup
+ path.gsub!('/', '')
if path['<<']
path.gsub!(/<<\s*(\w*)/) { "from-#$1" }
end
--
1.6.5.2

[Luke Kanies]

So this will just be writing the actual regex as the filename? Is it
worth cleaning that regex up at all or anything, so chars like '*'
aren't put into files? Or do we just not worry about it?

> --
>
> You received this message because you are subscribed to the Google
> Groups "Puppet Developers" group.
> To post to this group, send email to puppe...@googlegroups.com.
> To unsubscribe from this group, send email to puppet-dev+...@googlegroups.com
> .
> For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=
> .
>
>


--
I'm worried about Bart. Today, he's sucking people's blood,
tommorrow he might be smoking. -Marge Simpson
---------------------------------------------------------------------
Luke Kanies | http://reductivelabs.com | http://madstop.com

[Brice Figureau]

On 22/11/09 19:46, Luke Kanies wrote:
> So this will just be writing the actual regex as the filename? Is it
> worth cleaning that regex up at all or anything, so chars like '*'
> aren't put into files? Or do we just not worry about it?
>

I decided to not worry about it after thinking quickly about it.
Technically I somewhat think you can stuff whatever character you want
on a filename (even /) on unix.

The issue I didn't think about is when you use a browser and a webserver
to access the pages. Some of the regexes might not be url-safe, but I
think browser are quite forgiving in this context.

I'll do a couple of tests to see what happens, and if that doesn't work,
I'll add a translation layer.
--
Brice Figureau
My Blog: http://www.masterzen.fr/

[Luke Kanies]

Sounds good. I don't want to get too anal about it, just figured I'd
ask. It's also a great sysadmin interview question: "How do you
remove a file starting with a '-'?"

--
No one who cannot rejoice in the discovery of his own mistakes
deserves to be called a scholar. --Donald Foster

[Brice Figureau]

On 23/11/09 07:24, Luke Kanies wrote:
> On Nov 22, 2009, at 1:59 PM, Brice Figureau wrote:
>
>> On 22/11/09 19:46, Luke Kanies wrote:
>>> So this will just be writing the actual regex as the filename? Is it
>>> worth cleaning that regex up at all or anything, so chars like '*'
>>> aren't put into files? Or do we just not worry about it?
>>>
>>
>> I decided to not worry about it after thinking quickly about it.
>> Technically I somewhat think you can stuff whatever character you want
>> on a filename (even /) on unix.
>>
>> The issue I didn't think about is when you use a browser and a
>> webserver
>> to access the pages. Some of the regexes might not be url-safe, but I
>> think browser are quite forgiving in this context.
>>
>> I'll do a couple of tests to see what happens, and if that doesn't
>> work,
>> I'll add a translation layer.
>
> Sounds good. I don't want to get too anal about it, just figured I'd
> ask. It's also a great sysadmin interview question: "How do you
> remove a file starting with a '-'?"

It looks like I was (once again) plain wrong :-)

I need to add a translation layer: it doesn't seem possible for
filenames to contain * or other +/? when addressed through a web-browser.

Please disregard the previous patch, I'll send a more complete one later
this week.

[Brice Figureau]

The problem is that regex node contains '/' which is a directory
separator on unix.
Since puppetdoc writes a file for each node this was creating empty
directories and documentation for such node couldn't be stored.
This patch removes the slashes in the node names.

Signed-off-by: Brice Figureau <brice-...@daysofwonder.com>
---

.../util/rdoc/generators/puppet_generator.rb | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/puppet/util/rdoc/generators/puppet_generator.rb b/lib/puppet/util/rdoc/generators/puppet_generator.rb
index bf2609f..58c0aca 100644
--- a/lib/puppet/util/rdoc/generators/puppet_generator.rb
+++ b/lib/puppet/util/rdoc/generators/puppet_generator.rb
@@ -1,5 +1,7 @@
require 'rdoc/generators/html_generator'
require 'puppet/util/rdoc/code_objects'
+require 'digest/md5'
+
module Generators

# This module holds all the classes needed to generate the HTML documentation
@@ -335,7 +337,7 @@ module Generators
resources.each do |r|
res << {
"name" => CGI.escapeHTML(r.name),
- "aref" => "#{path_prefix}\##{r.aref}"
+ "aref" => CGI.escape(path_prefix)+"\#"+CGI.escape(r.aref)
}
end
res
@@ -414,7 +416,7 @@ module Generators

if path['<<']
path.gsub!(/<<\s*(\w*)/) { "from-#$1" }
end

- File.join(prefix, path.split("::")) + ".html"
+ File.join(prefix, path.split("::").collect { |p| Digest::MD5.hexdigest(p) }) + ".html"
end

def parent_name
@@ -508,7 +510,7 @@ module Generators
h_name = CGI.escapeHTML(name)

@values["classmod"] = "Node"
- @values["title"] = "#{@values['classmod']}: #{h_name}"
+ @values["title"] = CGI.escapeHTML("#{@values['classmod']}: #{h_name}")

c = @context
c = c.parent while c and !c.diagram
--
1.6.5.2

[Brice Figureau]

On 24/11/09 20:04, Brice Figureau wrote:
> The problem is that regex node contains '/' which is a directory
> separator on unix.
> Since puppetdoc writes a file for each node this was creating empty
> directories and documentation for such node couldn't be stored.
> This patch removes the slashes in the node names.

Technically, I don't remove the /, but write md5(nodename) as the file
name. This isn't perfect and is a 2 minutes hack, but I don't think
there'll be some collision.

[Markus Roberts]

Those are going to be some big ugly names there.  Any reason we can't just escape problematic characters and leave the majority human readable?  Maybe something like:

    File.join(prefix, path.split("::").collect { |p| p.gsub(/([^a-zA-Z0-9_-])/) { "%#{$1[0].to_i}%" }  }) + ".html"

-- Markus

--

You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to puppe...@googlegroups.com.
To unsubscribe from this group, send email to puppet-dev+...@googlegroups.com.

For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.

[Brice Figureau]

Reposting as my reply went to Markus only...

On 24/11/09 20:48, Markus Roberts wrote:
> Those are going to be some big ugly names there. Any reason we can't
> just escape problematic characters and leave the majority human
> readable? Maybe something like:

Correct, but those are not visible to human, except if they browse the
directory where the nodes are stored so I thought we didn't care.
The don't even appear as url in the browser because of the frameset.

> File.join(prefix, path.split("::").collect { |p|
> p.gsub(/([^a-zA-Z0-9_-])/) { "%#{$1[0].to_i}%" } }) + ".html"

Hmm, if we escape we'll have to double escape when used as url.
I personally don't have any preferences, any other comment?

[Markus Roberts]

If no one ever sees them (and we clean them up ourselves) I don't have strong feelings.

[Luke Kanies]

I concur; take the functional patch and move on.

--
Should I say "I believe in physics", or "I know that physics is true"?
-- Ludwig Wittgenstein, On Certainty, 602.