Fixed a bug preventing logging

[a7a00]

Thanks to the magic of UNIX, when we attempt to make log files for channels that contain slashes, we get an error as we try to create a directory that doesn't exist.

I just replaced the / with the similar looking, but friendlier to the shell %.

[astorije]

Hi @lyra833, thanks a lot for your contribution!

I'm thinking edge cases here, have you tested what happens if you join both #channel/name and #channel%name? I'm guessing there is either a perpetual override or the logs are being mangled together in the same file.

[a7a00]

Of course! I'll look into that right now.

[a7a00]

Obviously prefixing existing %'s with another % not the most elegant solution, but it works as a stopgap for now. I don't know if this is an important enough issue to cause a notification to the user, though.

If you think it is, I can make a nice notification and wrap it up as another PR.

[a7a00]

Is there anything else I should do?

[astorije]

Could you squash your commits and reword your message according to the guidelines in the CONTRIBUTING file, please?

Please also add the following comment above your change:

        // Quick fix to escape pre-escape channel names that contain % using %%,
        // and / using %. **This does not escape all reserved words**

That said, even if I +1 this quick solution, this is not a comprehensive fix. There are so many things to check and escape that it goes far beyond that.
We essentially need a library that does something similar to sanitize-filename, but instead of just removing the reserved words we would need to escape them and to be able to un-escape them.
I couldn't find such library however, which is why I would be OK with this fix until someone is willing to create one.

Also, if you would decide to add tests for this bug and similar others, that would be truly awesome :-)

In the meantime, if another maintainer can give his second opinion, that'd be appreciated.

[a7a00]

I'm not too sure what sort of comprehensive testing we need until the scope expands.

[astorije]

What do you mean by "until the scope expands"? We can at least record the initial bug as a test so that we don't have regressions.
In the future, I'd like these sorts of things to be tested against part or all of the Big List of Naugthy Strings.

Re: commit message, you have probably missed that part of the CONTRIBUTING file:

The general rules are to use the imperative present tense, to start with a single concise line, followed by a blank line and a more detailed explanation when necessary. Tim Pope wrote an excellent article on how one should format their commit messages.

[astorije]

@lyra833, ping?

[a7a00]

Sorry, massive school project. I'll try to get this fixed up by tomorrow.

[astorije]

I don't want to block this for a commit message format issue, so I'm 👍 on this (but if @lyra833 stops by here to edit, I wouldn't be against :P).

May I have a second review on this? @JocelynDelalande, @erming, @floogulinc?

[JocelynDelalande]

That's a 👍 for me also.

@lyra833 when you mean say that not all reserved words are checked, you mean "." and ".." I guess ? (they seem harmless in this case)