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

Support tput cols on Windows #424

Merged
merged 3 commits into from
Sep 24, 2024
Merged

Support tput cols on Windows #424

merged 3 commits into from
Sep 24, 2024

Conversation

DRSDavidSoft
Copy link
Contributor

Description

This PR allows Kint to detect the terminal cols on CLI, even on Windows.

It isn't fair to assume tput isn't available on Windows. It's part of the ncurses package and can be installed on many platforms, on Windows it isn't installed by default, but Cygwin, MSYS 2 (think Git for Windows) and many more might provide it.

It's better to call it, you are already handling the STDERR by redirecting it to the /dev/null, on Windows it needs to be redirected to NUL instead.

This way, if the package is not installed, nothing changes. But now, if the binary exists, it will be used.


n.b. Many developers on Windows will set-up the environment in a way to make the development easier/hassle free. Example is using Laragon, which is an all-in-one development environment suited for PHP developers. It includes Cmder, a portable terminal emulator that is designed to give Unix-like feeling to the users, through the use of clink, which is a powerful bash-like shell for Windows.

Now, either the user has installed the Git for Windows project, which already provides a tput command to determine the width, or they might choose to install the Cmder Full edition, which has a vendored Git for Windows bundled in it.

This means that many devs already have tput installed on their machines (or can do so with a simple Git for Windows installation) which also adds it to the path.

This way, now Kint can make use of this capability even on Windows.

@DRSDavidSoft
Copy link
Contributor Author

I couldn't get Composer to build the same file as the CI. Maybe the difference is because of using Windows.

@jnvsor
Copy link
Member

jnvsor commented Sep 24, 2024

Maybe the difference is because of using Windows

It definitely could be, but it probably shouldn't. I'll have to take a look.

How does this interact with the sapi_windows_vt100_support check earlier in the code? Should the check in the first if be changed to !$this->windows_output instead of !KINT_WIN?

@@ -92,9 +92,9 @@ public function __construct()
}

if (null === self::$terminal_width) {
if (!KINT_WIN && self::$detect_width) {
if (self::$detect_width) {
Copy link
Member

@jnvsor jnvsor Sep 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be !$this->windows_output && self::$detect_width instead? That'd guard against windows terminals that don't have sapi_windows_vt100_support but I'm not sure that coincides with the presence of tput

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am investigating this. Will run a Windows 7 terminal for testing purposes. While I'm on it, I would like to also address the lack of colors on Windows, it seems to be related to this.

@jnvsor
Copy link
Member

jnvsor commented Sep 24, 2024

diff --git a/build.php b/build.php
index 6ab0edf0..b06fe7a8 100644
--- a/build.php
+++ b/build.php
@@ -56,6 +56,7 @@ $filesToArchive = Finder::create()
     ->sortByName();
 
 foreach ($filesToArchive as $file) {
+    echo ((string) $file).PHP_EOL;
     $local = \substr((string) $file, $pathlen);
     $phar->addFile((string) $file, $local);
     $pi = new PharFileInfo('phar://'.$outpath.$local);

It looks like the files are being added to the phar in a different order on windows. Can you apply the above patch to build.php and paste me the output? Might be able to fix the reproducible build on windows too then.

@jnvsor jnvsor merged commit 52cdca6 into kint-php:master Sep 24, 2024
17 checks passed
@DRSDavidSoft DRSDavidSoft deleted the win-tput branch September 24, 2024 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants