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

Wrapped line is preserved even if nothing is written before executing #6826

Closed
mqudsi opened this issue Mar 29, 2020 · 4 comments
Closed

Wrapped line is preserved even if nothing is written before executing #6826

mqudsi opened this issue Mar 29, 2020 · 4 comments
Labels
bug Something that's not working as intended
Milestone

Comments

@mqudsi
Copy link
Contributor

mqudsi commented Mar 29, 2020

This one is subtle and the title is confusing. Sorry!

Given a terminal that is 80 characters wide, if 79 characters are entered at the prompt (including the prompt itself), the caret/cursor is now in the 80th column and no wrapping is triggered. Type in one more character and the caret moves to the start of the next line in anticipation of your input. All well and good.

If at this point you choose to execute the command as-is rather than append more text, this wrapped line is preserved in the output. This will appear to be an additional line of output rather than an extra line of input (especially if you typed it out quickly and hit enter knowing you were done, i.e. didn't wait for the cursor to move to the next line after entering a character in the 80th column).

e.g. the last two commands in the screenshot below actually produced identical output, but you wouldn't be able to tell that:

fish_line_wrapping

I believe we explicitly move the cursor to the next line regardless of the default terminal behavior/flags, right?

@krobelus krobelus added the bug Something that's not working as intended label Mar 30, 2020
@krobelus krobelus added this to the fish-future milestone Mar 30, 2020
@zanchey
Copy link
Member

zanchey commented Apr 16, 2020

I've actually noticed this before! I think it happens in other shells too.

@krobelus
Copy link
Member

Interestingly, it seems to happen in zsh, and bash when invoked as sh, but not in normal bash (with or without --posix) or in dash.

@fratajczak
Copy link

fratajczak commented Apr 27, 2020

The newline comes from this line in reader.cpp:

ignore_result(write(STDOUT_FILENO, "\n", 1));

Checking if the cursor position onscreen is a multiple of the screen width before writing the newline would fix it. However I didn't find a way to get the actual position of the cursor, only the position of the cursor without taking the prompt into account.
Inserting a char to see if the line count increases works, but it's a bit cumbersome and requires a repaint:

	size_t old_line_count = screen.actual.line_count();
	insert_char(active_edit_line(), L'a');
	size_t new_line_count = screen.actual.line_count();
	delete_char();
	repaint();
	if (old_line_count == new_line_count){
		ignore_result(write(STDOUT_FILENO, "\n", 1));
	}

(this also triggers #6951)
Is there a better way?

@ridiculousfish
Copy link
Member

There's a few subtleties here:

  1. Of course the cursor does not have to be positioned at the end of the command when you hit return.

  2. There's a risk that, if we do not emit a newline and instead rely on soft-wrap behavior, copy-and-paste of the terminal contents will discard the soft-wrapped newline. It seems at least iTerm2 does not have this issue.

I think we can still do this safely - we'll see if anyone spots a regression. Fixed in 4f103d7

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something that's not working as intended
Projects
None yet
Development

No branches or pull requests

5 participants