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

COLUMNS is set to 80 rather than terminal width #272

Closed
yorkshire-pudding opened this issue Nov 14, 2024 · 9 comments · Fixed by #283
Closed

COLUMNS is set to 80 rather than terminal width #272

yorkshire-pudding opened this issue Nov 14, 2024 · 9 comments · Fixed by #283
Labels
bug Something isn't working

Comments

@yorkshire-pudding
Copy link

I thought from lando/lando#1847 that Lando would get columns from the terminal.
I am running Ubuntu 24.04.1 LTS within WSL2 using Windows Terminal

Docker version:

$ docker --version
Docker version 27.3.1, build ce12230

If I do:

services:
  appserver:
    overrides:
      environment:
        COLUMNS: "140"

then it picks it up and works fine, but if not, it is stuck at 80, which IMHO is worse than the 256 it was

@yorkshire-pudding yorkshire-pudding added the bug Something isn't working label Nov 14, 2024
@pirog
Copy link
Member

pirog commented Nov 15, 2024

@yorkshire-pudding to be clear it was determined in lando/lando#1847 that lando should no longer manage this.

As far as i understand it Docker now automatically sets this based on your terminal width and that has personally been my experience. I think an exception to that would be anywhere where COLUMNS is explicitly set in either a container or potentially on the host as well.

cc @AaronFeledy for additional input

@yorkshire-pudding
Copy link
Author

Thanks @pirog - that's what I thought too, so I guess the issue will be with Docker rather than Lando; at least Lando has a way I can set it.
I wonder if the combination of WSL2 and Windows Terminal somehow prevents Docker from detecting it. Interested if you have any other thoughts but I'll perhaps search to see if there is a Docker config that I need to set.

@AaronFeledy
Copy link
Member

AaronFeledy commented Nov 15, 2024

❯ lando ssh -s appserver -c "tput cols"
291

❯ lando exec appserver -- "tput cols"
80

❯ /usr/bin/docker exec --tty --interactive --workdir=/app --user=www-data testdrupal9_appserver_1 tput cols
291

There seems to be some inconsistency. I'm looking into it.

@pirog
Copy link
Member

pirog commented Nov 15, 2024

ahh interesting! i bet exec does not pass in the same stdio options as ssh

@yorkshire-pudding
Copy link
Author

To give some context I'm coming across this in Bee, the command line tool for Backdrop CMS (now part of the default Backdrop CMS recipe); I'm also the maintainer of the tool so probably find more issues than most in my testing.

@pirog
Copy link
Member

pirog commented Nov 15, 2024

@yorkshire-pudding what are the actual commands you are running to see this?
I can definitely replicate what @AaronFeledy posted but how do i replicate your situation?

@yorkshire-pudding
Copy link
Author

yorkshire-pudding commented Nov 15, 2024

I'm running lando bee projects --debug. I got errors when I had some modules with long names and long version strings, but using the debug option will show the width that the tool is getting.

Bee gets terminal width using the PHP command:

$terminal_width = exec('if tput cols &>/dev/null; then  echo $(tput cols); else $(echo $COLUMNS); fi');

@pirog
Copy link
Member

pirog commented Nov 15, 2024

k thanks

pirog added a commit that referenced this issue Dec 2, 2024
…eriting terminal columns and lines correctly
@pirog pirog linked a pull request Dec 2, 2024 that will close this issue
pirog added a commit that referenced this issue Dec 2, 2024
…eriting terminal columns and lines correctly
@pirog pirog closed this as completed in #283 Dec 2, 2024
@yorkshire-pudding
Copy link
Author

Thanks Mike @pirog . Much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants