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

Feature request: Support full-path PAGER on native Windows #185

Closed
kit-ty-kate opened this issue Feb 5, 2024 · 11 comments
Closed

Feature request: Support full-path PAGER on native Windows #185

kit-ty-kate opened this issue Feb 5, 2024 · 11 comments

Comments

@kit-ty-kate
Copy link
Contributor

Currently setting the PAGER environment variable on Windows to the full-path to the binary (e.g. C:\cygwin64\bin\groff.exe) does not work as cmdliner uses the where cmd.exe command which does not support full-paths unlike command -v used on Unix.

Discussed in ocaml/opam#5797

@dbuenzli
Copy link
Owner

dbuenzli commented Feb 5, 2024

So what I'm supposed to do ? First check Filename.is_relative on the value and if false use where ?

@kit-ty-kate
Copy link
Contributor Author

I would personally do something like:

if String.equal (Filename.basename value) value then
  Sys.command "where ..."
else if Sys.file_exists value then
  0
else
  1

to emulate the command -v behaviour but I'm a Windows beginner and I'm not too picky anyway.

@dbuenzli
Copy link
Owner

dbuenzli commented Feb 5, 2024

Tried something. Tell me if it doesn't work.

@kit-ty-kate
Copy link
Contributor Author

I've done some tests:

  • Paging now works out of the box in Cygwin terminal (when Sys.win32 = true)
  • Paging doesn't work out of the box on Cmd and Powershell because TERM is not set by default (in which case cmdliner bypass the pager check entirely and always render things in plain text)
  • When forcing a non-plain mode (with --help=pager) with PAGER=C:\Cygwin64\bin\less.exe it works on both Cmd and Powershell
  • However there is a slight glitch on Cmd where the UTF8 is not rendered correctly. It displays ÔÇa (with a underlined, small and above other letters) instead.

@dbuenzli
Copy link
Owner

Thanks for the tests. 5292a8c tweaks the logic to always page on Sys.win32 = true and `Auto.

It displays ÔÇa

Ô ça j'en sais rien. IIRC I don't think cmd.exe actually supports UTF-8 by default. Random stackexchange answer.

@dbuenzli
Copy link
Owner

Ah but wait Sys.win32 is also true on cygwin we may want to still have the TERM lookup there.

@kit-ty-kate
Copy link
Contributor Author

Ô ça j'en sais rien. IIRC I don't think cmd.exe actually supports UTF-8 by default. Random stackexchange answer.

From a discussion with @dra27 this is apparently related to ocaml/ocaml#1408

Ah but wait Sys.win32 is also true on cygwin we may want to still have the TERM lookup there.

I agree

@dbuenzli
Copy link
Owner

Now that Windowzing is all the rage, are we subtle enough ?

@kit-ty-kate
Copy link
Contributor Author

I haven't had time to test cmdliner master, I'll do that right now, give me a second

@kit-ty-kate
Copy link
Contributor Author

kit-ty-kate commented May 22, 2024

So after testing, before and after, paging now works perfectly out of the box with Cygwin Terminal/bash, Windows Terminal/PowerShell and Command Prompt/cmd with cmdliner master (paged by less on Cygwin and by more.com on Windows Terminal and cmd)

However on both Windows Terminal/PowerShell and Command Prompt/cmd we do get the weird UTF-8 characters still.
Looking a bit into the matter, executing chcp 65001 (found both in the above stackexchange and this stackoverflow thread) seems to fix the issue for both cmd and powershell. I'm not sure if there is anything we can do anything in cmdliner while waiting for ocaml/ocaml#1408 but at least opam could add the command in our release announcement.

I think we'd be happy to get a release of cmdliner that we can use before the release of opam 2.2.0~beta3 next week or before 2.2.0~rc1, but it's absolutely not critical so no rush ^^

Sorry for the delay on this.

@dbuenzli
Copy link
Owner

Thanks for feedback. Will do a release, I'm eager to use d203fb4 in my code bases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants