Make diffing work on Windows #904
Conversation
ghost
requested a review
from 
|
@diml is this WIP? |
ghost
changed the title
|
nope, I forgot to change the title |
|
Thanks, I've tested it and it seems to work fine ! |
src/action.ml
Outdated
| ; "diff?", | ||
| (path >>= fun file1 -> | ||
| path >>= fun file2 -> | ||
| Syntax.get_exn Stanza.syntax >>| fun ver -> | ||
| let mode = if ver < (1, 0) then Diff_mode.Text_jbuild else Text in |
There was a problem hiding this comment.
This idiom should be extracted IMO. Something like Syntax.choose syntax ~jbuild:... ~dune:....
There was a problem hiding this comment.
Agreeed. I added Stanza.file_kind, the advantage compared to choose is that it cost less when the two values require computation.
| Bytes.blit_string src src_pos dst dst_pos len; | ||
| let dst_pos = dst_pos + len in | ||
| Bytes.set dst dst_pos '\n'; | ||
| loop (i + 2) (dst_pos + 1) |
There was a problem hiding this comment.
Did you give writing this a code try with mutual recursion style? I find that much easier to read usually. I'm pretty certain that you did, since I basically learned this technique from you, but the current code is a bit hard to follow. Anyway, I've convinced myself that this is correct so approving
There was a problem hiding this comment.
I think I did but there were a lot of variables to maintain so I thought it was better this way
- make (diff ...) trailing cr on Win32 - add a (cmp ...) action for comparing binary files - add a test and run it in AppVeyor Fix #844 Signed-off-by: Jeremie Dimino <jeremie@dimino.org>
Signed-off-by: Jeremie Dimino <jeremie@dimino.org>
ghost
mentioned this pull request
This PR changes the behavior of
(diff a b)on Windows. Currently, it is common to compare a generated file against a source one. However, it is often the case that on Windows the generated file contains lines ending with\r\n, which causes the comparison to fail if the source file has unix end of lines.After this PR,
(diff a b)normalizes the end of lines on Windows before comparing the files. Additionally, it passes--skip-trailing-crtodiffon Windows so thatdiffdoes the same comparison as Dune.In some cases,
diffmight be used to compare binary files. For this purpose acmpaction was added.(cmp a b)always compare the raw contents ofaandband never callsdiff. Instead it simply prints:Promoted files are still promoted as it. It's not clear what Dune should do but I expect that people working on Windows configure their git so that files are checkouted with Windows end of lines.