Windows compatibility #47
Conversation
- Replaced unicode chatacter → in prompt as it causes problems in standard windows terminals - created putStrFlush to be used instead of T.putStr, ensure content is flushed to the console and prompts are displayed in the correct order - replaced use of "rm" with System.Directory removeFile. Wrapped in a function deleteFile to continue on error - modify setScriptCommand to do nothing on windows systems
|
Wow! @puhlenbruck thanks for contributing to |
|
@vrom911 Thanks! Please do let me know if there are any suggestions or changes you would like to see before merging. |
There was a problem hiding this comment.
Wow! That's really cool work done here! Sorry I couldn't test it with windows, but I'm sure you did it 🙂 Still I checked it on my machine and everything seems good!
That's a pity that unicode symbols cause fails on windows, but there is nothing that can be done with that.. I am thinking to improve --help section as it uses a looot of unicode symbols.
Also thanks for another thoughts you added, I will create few issues on that to think about more about. Though there is already one about the colors : #28 . When it implemented it will help a lot, but anyway I will add comment there.
Again, thanks a lot for your time!
hs-init.hs
Outdated
|
|
||
| deleteFile :: FilePath -> IO () | ||
| deleteFile file = catch (removeFile file) printError | ||
| where printError (e :: SomeException) = errorMessage $ "Could not delete file `" <> T.pack file <> "'. " <> T.pack (displayException e) |
There was a problem hiding this comment.
I see that back-ticks are mismatching in here.
hs-init.hs
Outdated
| @@ -442,14 +448,14 @@ reset = setSGR [Reset] | |||
| prompt :: IO Text | |||
| prompt = do | |||
| setColor Blue | |||
| T.putStr " → " | |||
| putStrFlush " -> " | |||
There was a problem hiding this comment.
Can I ask you to make here 2 spaces before the arrow, I think it adds a little bit more readability 🙂
hs-init.hs
Outdated
| fromString cmd args = callProcess cmd (map T.unpack args) | ||
|
|
||
| deleteFile :: FilePath -> IO () | ||
| deleteFile file = catch (removeFile file) printError |
There was a problem hiding this comment.
I like this solution! Great! I think we can continue and this trash file that didn't removed is not the reason to stop the process
There was a problem hiding this comment.
In case there is confusion, this function does allow the process to continue simply printing a message if the delete fails, but not stopping the process.
hs-init.hs
Outdated
| @@ -430,6 +431,11 @@ falseMessage target = False <$ warningMessage (T.toTitle target <> " won't be ad | |||
| -- Ansi-terminal | |||
| ---------------------------------------------------------------------------- | |||
|
|
|||
| putStrFlush :: Text -> IO() | |||
There was a problem hiding this comment.
I think you forgot to add space before (
| @@ -508,7 +514,11 @@ customizeLicense l t nm year | |||
| -- This is needed to be able to call commands by writing strings. | |||
| instance (a ~ Text, b ~ ()) => IsString ([a] -> IO b) where | |||
| fromString "cd" [arg] = setCurrentDirectory $ T.unpack arg | |||
| fromString cmd args = callCommand $ showCommandForUser cmd (map T.unpack args) | |||
| fromString cmd args = callProcess cmd (map T.unpack args) | |||
There was a problem hiding this comment.
Could you please clarify the reason for changing callCommand showCommandForUser with callProcess?
There was a problem hiding this comment.
Despite the docs claiming otherwise, showCommandForUser doesn't actually produce a string that works on windows. The problem seems similar to haskell/process#51
|
@puhlenbruck Regarding other issues you shared:
Well, I guess we don't need to create |
|
Ahh, also about this PR: I think you should also add information to changelog regarding your changes 👍 Because your changes are great 🙂 |
|
Thank you for the feedback, I'll look to make these changes. @vrom911 Yes it's unfortunate about unicode. There seem to be some potential solutions I might look into in the future but for now I'm working as if it's not possible. With this PR I'm just looking to get to a minimal working state. Not too concerned with the help page yet, it still works and shows the important information, just the graphics are a bit mangled. @chshersh If |
Corrected typos Added CHANGELOG.md entry install.ps1 now checks to see if the install directory is already on the path before adding it added explicit flush to ansi-terminal reset added comment string on new functions.
hs-init.hs
Outdated
| fromString cmd args = callProcess cmd (map T.unpack args) | ||
|
|
||
| deleteFile :: FilePath -> IO () | ||
| deleteFile file = catch (removeFile file) printError |
|
@puhlenbruck Thanks again for your wonderful contribution! This makes me so happy that |
Crazies like me like to work on windows sometimes. This pr introduces a few changes to help improve the windows compatibility.
Added a powershell install script and windows install instruction to README
Replaced unicode chatacter → in prompt as it causes problems in
standard windows terminals
created putStrFlush to be used instead of T.putStr, ensure content is
flushed to the console and prompts are displayed in the correct order
replaced use of "rm" with System.Directory removeFile. Wrapped in a
function deleteFile to continue on error
modify setScriptCommand to do nothing on windows systems
These changes should provide a basic level of support for windows platforms. There are still some outstanding issues which are not addressed by this change:
The build script feature still only produces a bash script. It could potentially produce a powershell script as well for windows.
The default background for the powershell console is blue, the prompt arrow colour is also blue here making it a bit hard to read, perhaps the colour scheme could be adjusted.
The tool still relies on curl to be installed for some functions. Systems are not guaranteed to have this installed (this potentially affects linux systems as well). For now I've added it to the prerequisite listing in the README.md, but maybe this download function could be done from the haskell code without requiring an external tool.