-
Notifications
You must be signed in to change notification settings - Fork 413
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Safe url naming #304
Safe url naming #304
Conversation
@@ -28,5 +30,9 @@ class SourceDeclaration | |||
def overview | |||
"#{abstract}\n\n#{discussion}".strip | |||
end | |||
|
|||
def filename | |||
FilenameCleaner.sanitize("#{name}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for string interpolation
That gem seems to bring in a bunch of dependencies... Are we sure active support doesn't just have a method for this we could use? |
As far as I have read, there is no built-in way of escaping strings so they are safe for the filesystem. url = "init(baseURL:sessionConfig:requestSerializer:responseSerializer:)"
url.gsub(/[^\w\.]/, '')
=> "initbaseURLsessionConfigrequestSerializerresponseSerializer" |
@segiddins do you have an opinion on the above? |
sure, using file-cleaner is fine if @jpsim is ok with it |
Seems that file-cleaner does not copy the input and hence changes the input. head :001 > require "filename_cleaner"
=> true
head :002 > name = "Global Variables"
=> "Global Variables"
head :003 > new_name = FilenameCleaner.sanitize(name, '_', false)
=> "Global_Variables"
head :004 > name
=> "Global_Variables" I'm not that familiar with Ruby but it seems odd to me that I'm responsible for sending in a copy. |
Unless I'm missing something, it does seem like |
My take on #146
Requires realm/jazzy-integration-specs#4 to pass build