Skip to content
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

Windows support #12

Closed
lzybkr opened this issue Sep 29, 2019 · 20 comments
Closed

Windows support #12

lzybkr opened this issue Sep 29, 2019 · 20 comments
Labels
help wanted Extra attention is needed

Comments

@lzybkr
Copy link
Contributor

lzybkr commented Sep 29, 2019

Is Windows support planned?

delta builds successfully if you remove the dependency on termios from Cargo.toml.

Trying it out though it seems there are a couple issues demonstrated in the screenshot below (which is captured from my 1 line change to get delta to build):

image

Note first line in the box around the context is off by 1.

But more worrisome - the actual diff in Cargo.toml isn't highlighted at all like the default pager would:

image

@dandavison
Copy link
Owner

dandavison commented Sep 30, 2019

Thanks @lzybkr! I'm definitely happy to support Windows, in collaboration with one or more people using delta on Windows. (I don't have access to a Windows development environment. There are free VMs available from Microsoft for testing, but the problem is it really needs someone who knows about the different terminal emulator and shell / dev environment options.) I did have a Windows CI build running, and I've reinstated it in response to this issue (0e0ea02).

Note first line in the box around the context is off by 1.

Right. I am not sure yet why this is. I'd be happy to participate in discussions and review code fixing things like this on Windows.

But more worrisome - the actual diff in Cargo.toml isn't highlighted at all like the default pager would:

OK, I suspect that this is because delta is using default colors that are not supported by your terminal emulator (i.e. your terminal emulator does not support arbitrary 24 bit colors). You can see the colors delta is using like this, and you can use the command line options it prints out to select different colors that will work with your terminal emulator:

$  delta --show-colors 
delta --theme="Monokai Extended" --minus-color="#3f01" --minus-emph-color="#901011" --plus-color="#0280" --plus-emph-color="#e7ce"

@dandavison dandavison added the help wanted Extra attention is needed label Sep 30, 2019
@lzybkr
Copy link
Contributor Author

lzybkr commented Sep 30, 2019

My terminal definitely supports 24 bit colors (I'm using the new Windows Terminal):

image

Based on your feedback, I thought delta might not have been setting the correct console mode, but I added the necessary code (thanks to the output_vt100 crate, but it didn't help.

Are there potential issues with LF vs. CRLF?

@dandavison
Copy link
Owner

dandavison commented Sep 30, 2019

the problem is it really needs someone who knows about the different terminal emulator and shell / dev environment options.

OK, I should have read your Github profile first :)

I'm happy to help with this. I've just released v 0.0.10, so Delta will now have Windows binaries built via Travis CI on the Releases page: https://github.com/dandavison/delta/releases/download/0.0.10/delta-0.0.10-x86_64-pc-windows-msvc.zip If get a moment to try this binary that would be fantastic; presumably it behaves the same as the one you're already using.

@dandavison
Copy link
Owner

dandavison commented Sep 30, 2019

Is it just the Cargo.toml which is failing to show red/green minus/plus colors, or are you not seeing any background colors from delta at all?

If you do an equivalent of the command below, do you see the same ANSI color escape codes that I'm seeing?

$ git diff | delta | cat -A
$
^[[34mCargo.toml^[[0m$
^[[34mM-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@^[[0m^[[34mM-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@^[[0m^[[34mM-bM-^TM-^P^[[0m$
^[[34m^[[38;2;248;248;242m ^[[38;2;249;38;114mstructopt^[[38;2;248;248;242m = ^[[38;2;230;219;116m"0.2.18"^[[0m ^[[34mM-bM-^TM-^B^[[0m$
^[[34mM-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@M-bM-^TM-^@^[[0m^[[34mM-bM-^TM-^X^[[0m$
^[[34m63^[[0m$
^[[38;2;248;248;242m ^[[38;2;249;38;114mstructopt-derive^[[38;2;248;248;242m = ^[[38;2;230;219;116m"0.2.18"^[[38;2;248;248;242m                                                  $
^[[38;2;248;248;242m ^[[38;2;249;38;114msyntect^[[38;2;248;248;242m = ^[[38;2;230;219;116m"3.2.0"^[[38;2;248;248;242m                                                            $
^[[38;2;248;248;242m ^[[38;2;249;38;114msyn^[[38;2;248;248;242m = ^[[38;2;230;219;116m"0.15.42"^[[38;2;248;248;242m                                                              $
^[[48;2;63;0;1m termios = "0.3.1"^[[48;2;63;0;1m                                                            $
^[[38;2;248;248;242m ^[[38;2;249;38;114mtextwrap^[[38;2;248;248;242m = ^[[38;2;230;219;116m"0.11.0"^[[38;2;248;248;242m                                                          $
^[[38;2;248;248;242m ^[[38;2;249;38;114mthread_local^[[38;2;248;248;242m = ^[[38;2;230;219;116m"0.3.6"^[[38;2;248;248;242m                                                       $
^[[38;2;248;248;242m ^[[38;2;249;38;114mucd-util^[[38;2;248;248;242m = ^[[38;2;230;219;116m"0.1.5"^[[38;2;248;248;242m                                                           $

@lzybkr
Copy link
Contributor Author

lzybkr commented Oct 1, 2019

So git diff | delta works nicely (modulo the off by one box).

It looks like the issue is less. Guessing now - but it's looking like the Windows version of less.exe is not 256/24 bit color aware somehow because the 8 color escape sequences seem to render correctly, but the extended colors do not, e.g.:

image

@dandavison
Copy link
Owner

I hope that the box drawing bug on Windows is fixed in 0.0.13 (#22). Thanks for pointing it out.

@lzybkr
Copy link
Contributor Author

lzybkr commented Oct 14, 2019

Yeah, seems like it's fixed. I have some patches for less here that seem to work, but I'll do some additional testing before opening a PR for the second commit.

I'm still seeing some minor issues, but I can probably just open up new issues for those when I've confirmed they are issues with delta and not with less or the Windows terminal.

@lzybkr
Copy link
Contributor Author

lzybkr commented Oct 15, 2019

In my fork of less, I created a release to share the binary less.exe with my fixes for anyone wanting to try it out - see here.

@lzybkr
Copy link
Contributor Author

lzybkr commented Oct 18, 2019

I'm guessing I've found another issue with less, but --commit-style box ends up displaying garbage on Windows. Note in the screenshot I tried the same thing under wsl and it looks fine:

image

@dandavison
Copy link
Owner

@lzybkr thanks very much for all your work here and related work on less. Is there anything I can add to the README that would help Windows users? E.g. terminal emulator / shell recommendations, whether they need a different version of less, etc.

@bajtos
Copy link

bajtos commented Apr 26, 2020

Hi, download link for Windows is pointing to delta version 0.15.0, which does not support recently introduced options like --color-only. When I try to build the latest (master) version via cargo build, I get the following error:

cargo build --release
   Compiling onig_sys v69.2.0
error: failed to run custom build command for `onig_sys v69.2.0`

Caused by:
  process didn't exit successfully: `C:\Users\bajtos\AppData\Local\Temp\delta\target\release\build\onig_sys-d651f94dacb32272\build-script-build` (exit code: 101)
--- stdout
cargo:warning=couldn't execute `llvm-config --prefix` (error: The system cannot find the file specified. (os error 2))
cargo:warning=set the LLVM_CONFIG_PATH environment variable to a valid `llvm-config` executable

--- stderr
thread 'main' panicked at 'Unable to find libclang: "couldn\'t find any valid shared libraries matching: [\'clang.dll\', \'libclang.dll\'], set the `LIBCLANG_PATH` environment variable to a path where one of these files can be found (invalid: [])"', C:\Users\bajtos\.cargo\registry\src\github.jparrowsec.cn-1ecc6299db9ec823\bindgen-0.50.1\src/lib.rs:1711:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

AFAICT, this problem is caused by (recent?) changes in onig_sys, as discussed in rust-onig/rust-onig#109 I am not sure if/when a fixed version of onig_sys will be published, is there any chance of downgrading delta's dependencies to use an older version of onig_sys that works on Windows too?

@dandavison
Copy link
Owner

Hi @bajtos thanks very much for looking into this. Let's sort this out. I have been aware that the Travis Windows builds were broken (I posted a question about it here on the Travis forums). I don't have a Windows machine locally. Is the best way for me to test just to try Travis Windows builds until they work?

@dandavison
Copy link
Owner

OK I tried downgrading downgrading to onig_sys 69.1.0, but it wasn't that. I looked into it more carefully and the cause was that Travis changed their Windows builds from stable-x86_64-pc-windows-msvc to stable-x86_64-pc-windows-gnu, and so the requested build target in my Travis config no longer matched what their system could build. (AIUI one thing this means is that their Windows builds were previously being done under Windows OS and are now being done under Linux).

In any case, fixed now in master, and I'll get a release out (and hopefully the Windows binary built in the new way will work fine on your system). Thanks for pinging me on this (and let me know if something I said there doesn't sound right).

@dandavison
Copy link
Owner

@dandavison dandavison changed the title Windows support? Windows support Apr 26, 2020
@lzybkr
Copy link
Contributor Author

lzybkr commented Apr 27, 2020

Note that the windows-gnu built binary is roughly 3x bigger than the windows-msvc built binary - maybe it includes debug info?

If you want to use the windows-msvc compiler, you could switch from Travis to GitHub Actions for CI.

@dandavison
Copy link
Owner

Thanks @lzybkr that's good to know, and I think it makes sense to switch to Github Actions.

I had a couple of questions for you:

  • Presumably the work you've done on less are just as relevant to bat as they are to delta; does that sound true to you? Any comments on the advice the bat README gives regarding less on Windows? https://github.com/sharkdp/bat/#paging

  • Could it make sense for the delta Windows zip archive to bundle your less version? Or otherwise to make this more convenient for Windows users?

@lzybkr
Copy link
Contributor Author

lzybkr commented Apr 27, 2020

My memory is hazy but I think I was using bat some before fixing less but I do recall some issues and it's possible I fixed those in less.

As for packaging - git finds the less.exe that it ships even if you prepend PATH with your own version, so really we need to get the git distribution to pick up a newer less.

I'm not sure when they'll release a new version, but my PR hasn't been merged yet - and I see there is a conflict I need to resolve before it can be merged.

@bajtos
Copy link

bajtos commented May 2, 2020

Thank you @dandavison for fixing Windows build!

I downloaded the new version 0.1.1 using the link you provided and also installed the patched version of less as offered by @lzybkr in #12 (comment) and all works as expected now 🎉

Thank you folks, you are awesome! ❤️

@dandavison
Copy link
Owner

Delta continues to have functioning Windows binaries (and these are back on the msvc toolchain now -- Travis seemed to flip to one and then back without explanation that I was aware of). I'm going to close this issue now since its core objective has been realized.

Related:

  1. Include new Chocolatey package for Windows in installation list 🚀 #197 is an open issue relating to providing a Chocolatey package with convenient out-of-the box support (including appropriate less) and has good discussion.

  2. It would great to document which combination of Windows terminal emulators can be used for Delta and generally to provide more step-by-step instructions and troubleshooting advice for Windows users.

@djdv
Copy link

djdv commented Aug 10, 2020

@dandavison

It would great to document which combination of Windows terminal emulators can be used for Delta

Chiming in to say that I use delta frequently within a pretty native Windows environment. Powershell 7 inside of the Microsoft Terminal. Building delta from master with Rust toolchain nightly-x86_64-pc-windows-msvc, and using the patched less.exe binary. As well as Microsoft's fork of git to store and read the delta config settings.
Everything seems to work as intended. 👌
Untitled

I also wrote a powershell function which wraps delta and other tools, to have find and replace with diff previews. I use it pretty often on both Windows and *nix systems inside powershell. It used to not render the arrow character on Windows for this
old

but that seems to no longer be the case (this may have been unrelated to delta though).
rplace

Edit: I had my colors wrong, changed the first screenshot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants