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

Replace tput command as it's not available on all OSes #14

Merged
merged 2 commits into from
Jul 29, 2023
Merged

Replace tput command as it's not available on all OSes #14

merged 2 commits into from
Jul 29, 2023

Conversation

digiservnet
Copy link
Contributor

@digiservnet digiservnet commented Jul 28, 2023

The tput command isn't available by default in the Alpine Linux official PHP docker images (at least) and this throws an InvalidArgumentException for truncation length.

This PR replaces the tput command with method calls from the Symfony Console Terminal class.

tput is used in the Terminal class to determine the number of rows and columns available, in particular used for truncating line length if available terminal columns are too low.

@jessarcher
Copy link
Member

Thanks!

I wonder if we could change the approach or add a fallback...

For example, stty size returns the lines and columns separated by a space on Linux, and we're already using stty elsewhere. Does that work in Alpine?

I'd just need a macOS friend to check whether that works and returns the same format for them, and then we could just drop tput altogether.

@digiservnet
Copy link
Contributor Author

digiservnet commented Jul 29, 2023

Tested and stty size works fine in Alpine. If this can be confirmed on a mac (from looking around on the interwebz, it does produce the same rows cols format), I can push the changes. 🙂

We can use mode CON on windows too (assuming you want to support windows?)

@digiservnet
Copy link
Contributor Author

Had confirmation that stty behaves the same on macOS so have gone with this approach.

Also added Windows detection and utilised mode there. The output from this is:

mode CON
Status for device CON:
----------------------
    Lines:          42
    Columns:        180
    Keyboard rate:  31
    Keyboard delay: 0
    Code page:      850

@digiservnet digiservnet changed the title Check for and require tput command to be available Replace tput command as it's not available on all OSes Jul 29, 2023
@wouterj
Copy link

wouterj commented Jul 29, 2023

Symfony Console, which you depend on in this library, provides as much compatibility as possible in the Terminal class. I think you should be able to directly use that class it in this library.

The tput command isn't available by default in the Alpine Linux official
PHP docker images and this throws an InvalidArgumentException for
truncation length.

This commit replaces tput in favour of using the Symfony `Terminal` 
class from Console, which is already part of the package.
@digiservnet
Copy link
Contributor Author

Good call @wouterj. I've replaced the custom code with the Symfony Terminal getWidth() and getHeight() method calls as there doesn't seem much reason to duplicate this functionality 🙂

@jessarcher
Copy link
Member

Thanks!

@jessarcher jessarcher merged commit eeae4d7 into laravel:main Jul 29, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants