Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

Optimized for Windows OS #4

Closed
wants to merge 1 commit into from
Closed

Conversation

moyamejiasr
Copy link
Contributor

The actual version breaks all the epub generated files on windows.
This little fix will repair most of the problems on it.

@coveralls
Copy link

coveralls commented Feb 16, 2018

Coverage Status

Coverage increased (+0.08%) to 81.354% when pulling 741728a on Onelio:master into b0e9199 on bmaupin:master.

@bmaupin
Copy link
Owner

bmaupin commented Feb 18, 2018

It looks like filepath.ToSlash might be a cleaner way of handling this:

Use the function filepath.ToSlash to replace the operating system path separator with '/' in a path.

On Windows, the function returns strings.Replace(path, string(filepath.Separator), "/", -1). On other operating systems, the function returns the path argument as is.

(https://stackoverflow.com/a/40002602/399105)

What do you think?

@moyamejiasr
Copy link
Contributor Author

Seems clearer for me also, use this one if you like.

@bmaupin
Copy link
Owner

bmaupin commented Feb 18, 2018

Okay, I'll reopen your pull request since I think I can just modify it.

@bmaupin bmaupin reopened this Feb 18, 2018
@moyamejiasr
Copy link
Contributor Author

moyamejiasr commented Feb 19, 2018

Looking it more deeply seems like the function does the same thing as my commit
https://golang.org/src/path/filepath/path.go#L165

bmaupin pushed a commit that referenced this pull request Feb 19, 2018
@bmaupin
Copy link
Owner

bmaupin commented Feb 19, 2018

I made the changes and pushed them to the windows-fixes branch. Can you make sure it works for you and then I'll merge? I don't have access to a Windows machine.

Thanks!

@moyamejiasr
Copy link
Contributor Author

It works nice

bmaupin pushed a commit that referenced this pull request Feb 20, 2018
@bmaupin
Copy link
Owner

bmaupin commented Feb 20, 2018

I've pushed the changes. Thanks for your contribution!

@bmaupin bmaupin closed this Feb 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants