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

[WIP] Feature mintty tui #8271

Closed
wants to merge 23 commits into from
Closed

[WIP] Feature mintty tui #8271

wants to merge 23 commits into from

Conversation

erw7
Copy link
Contributor

@erw7 erw7 commented Apr 13, 2018

This is an experimental implementation of mintty TUI which took over #7660.

After executeing, the state of the terminal does not return completely. Therefore drawing may be very slow. Please use reset or stty to restore the state of the terminal.

  • Resolved issue where tcgetattr() returns struct __oldtermios (This may be the cause of the terminal not returning completely to its original state).
  • Investigate the reason why uv_write fails to write to a pipe when nbufs>=2.
  • Confirm whether we can compile in environments other than MinGW (64 bit).
  • Implementation of error handling.
  • Confirm that there are no other problems.

@erw7 erw7 mentioned this pull request Apr 13, 2018
@marvim marvim added the WIP label Apr 13, 2018
// https://fossies.org/linux/vim/src/iscygpty.c
// See https://github.com/BurntSushi/ripgrep/issues/94#issuecomment-261745480
// for an explanation on why this works
int query_mintty(int fd, MinttyQueryType query_type)
Copy link
Member

Choose a reason for hiding this comment

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

small note about style: functions in os/* should be prefixed with os_.

- Rename function.
- Add Cygwindll struct.
- Change function used only in cygterm.c to static function.
- Fix that the termination process was not called.
- Correct warning.
- Fix lint issue.
- Add comment.
@janlazo
Copy link
Contributor

janlazo commented Apr 20, 2018

Does bash -c nvim work now so that nvim can run old-style tests on sh/bash?
This assumes that TERM is unset so that sh/bash can set it to TERM=cygwin.

@justinmk
Copy link
Member

This assumes that TERM is unset so that sh/bash can set it to TERM=cygwin.

Is there a terminfo file for "cygwin" ?

@janlazo
Copy link
Contributor

janlazo commented Apr 21, 2018

I have /usr/share/terminfo/63/cygwin for msysgit on my machine. It's the only file in that folder while the other folders (64/, 78/) have dumb, or variants of xterm.

> git --version
git version 2.15.0.windows.1

The cause of tcgetattr() returning __oldtermios was that API version of
Cygwin was not set.
- Remove unnecessary definition.
- Change not to expose to externally the inside of sturct CygTerm.
- Move some definition to source file from header.
kNoneMintty,
kMinttyCygwin,
kMinttyMsys
} MinttyType;
Copy link
Member

Choose a reason for hiding this comment

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

why kNoneMintty instead of kMinttyNone ?

@erw7
Copy link
Contributor Author

erw7 commented Apr 28, 2018

@janlazo This PR is to work nvim on Mintty. I think that you would like to run the old-style test on the console(cmd.exe). Therefore, I think this PR is irrelevant. bash -c nvim will work on the console even without this PR.

@justinmk
Copy link
Member

justinmk commented May 16, 2018

Can the user set on startup via init.vim so that the user doesn't have to use a shell script or batchfile to set environment variables prior to running nvim?

Check where ui_builtin_start is called from main.c .

Edit: It happens after source_startup_scripts so init.vim should work to set TERMINFO.

@janlazo
Copy link
Contributor

janlazo commented May 17, 2018

Can the patch to prevent abnormal termination because of TERM be cherry-picked for 0.3?

@justinmk
Copy link
Member

sure, can someone cherry-pick it to a new PR?

@erw7
Copy link
Contributor Author

erw7 commented May 18, 2018

I have created a new pull request that only fix issue with TERM.

@janlazo I think TUI display on WIndows 7 before Windows 8.1 will improve, by adding cygwin to the built-in TERMINFO and using it. But I think there is a possibility that xtem-256colors is better on Windows 10.

@janlazo
Copy link
Contributor

janlazo commented May 26, 2018

nvim TUI is usable on bash with the TERM fix. Scrolling is slow with non-default colorscheme. Which terminfo is nvim -u NORC using with TERM=cygwin?

@justinmk
Copy link
Member

@janlazo try infocmp, it should show the terminfo file at the top of its output:

infocmp -L

@janlazo
Copy link
Contributor

janlazo commented May 26, 2018

@justinmk I asked because I thought it's using a builtin-in terminfo.

> infocmp -L
#	Reconstructed via infocmp from file: /usr/share/terminfo/63/cygwin
cygwin|ansi emulation for Cygwin,
	auto_right_margin, has_status_line, move_insert_mode,
	move_standout_mode, xon_xoff,
	init_tabs#8, max_colors#8, max_pairs#64,
	acs_chars=+^P\054^Q-^X.^Y0\333`^Da\261f\370g\361h\260j\331k\277l\332m\300n\305o~p\304q\304r\304s_t\303u\264v\301w\302x\263y\363z\362{\343|\330}\234~\376,
	bell=^G, carriage_return=\r, clear_screen=\E[H\E[J,
	clr_bol=\E[1K, clr_eol=\E[K, clr_eos=\E[J,
	column_address=\E[%i%p1%dG,
	cursor_address=\E[%i%p1%d;%p2%dH, cursor_down=\E[B,
	cursor_home=\E[H, cursor_left=^H, cursor_right=\E[C,
	cursor_up=\E[A, delete_character=\E[P,
	delete_line=\E[M, enter_alt_charset_mode=\E[11m,
	enter_bold_mode=\E[1m, enter_ca_mode=\E7\E[?47h,
	enter_insert_mode=\E[4h,
	enter_pc_charset_mode=\E[11m,
	enter_reverse_mode=\E[7m, enter_secure_mode=\E[8m,
	enter_standout_mode=\E[7m,
	enter_underline_mode=\E[4m,
	exit_alt_charset_mode=\E[10m,
	exit_attribute_mode=\E[0;10m,
	exit_ca_mode=\E[2J\E[?47l\E8, exit_insert_mode=\E[4l,
	exit_pc_charset_mode=\E[10m,
	exit_standout_mode=\E[27m,
	exit_underline_mode=\E[24m, from_status_line=^G,
	insert_character=\E[@, insert_line=\E[L, key_b2=\E[G,
	key_backspace=^H, key_dc=\E[3~, key_down=\E[B,
	key_end=\E[4~, key_f1=\E[[A, key_f10=\E[21~,
	key_f11=\E[23~, key_f12=\E[24~, key_f13=\E[25~,
	key_f14=\E[26~, key_f15=\E[28~, key_f16=\E[29~,
	key_f17=\E[31~, key_f18=\E[32~, key_f19=\E[33~,
	key_f2=\E[[B, key_f20=\E[34~, key_f3=\E[[C, key_f4=\E[[D,
	key_f5=\E[[E, key_f6=\E[17~, key_f7=\E[18~,
	key_f8=\E[19~, key_f9=\E[20~, key_home=\E[1~,
	key_ic=\E[2~, key_left=\E[D, key_npage=\E[6~,
	key_ppage=\E[5~, key_right=\E[C, key_suspend=^Z,
	key_up=\E[A, newline=\r\n, orig_pair=\E[39;49m,
	parm_dch=\E[%p1%dP, parm_delete_line=\E[%p1%dM,
	parm_down_cursor=\E[%p1%dB, parm_ich=\E[%p1%d@,
	parm_insert_line=\E[%p1%dL,
	parm_left_cursor=\E[%p1%dD,
	parm_right_cursor=\E[%p1%dC,
	parm_up_cursor=\E[%p1%dA, reset_1string=\Ec\E]R,
	restore_cursor=\E8, row_address=\E[%i%p1%dd,
	save_cursor=\E7, scroll_forward=\n, scroll_reverse=\EM,
	set_a_background=\E[4%p1%dm,
	set_a_foreground=\E[3%p1%dm,
	set_attributes=\E[0;10%?%p1%t;7%;%?%p2%t;4%;%?%p3%t;7%;%?%p6%t;1%;%?%p7%t;8%;%?%p9%t;11%;m,
	tab=^I, to_status_line=\E];, user6=\E[%i%d;%dR,
	user7=\E[6n, user8=\E[?6c, user9=\E[c,

@justinmk
Copy link
Member

@janlazo There's no builtin for cygwin (yet), see terminfo_builtin:

static unibi_term *terminfo_builtin(const char *term, char **termname)

@erw7
Copy link
Contributor Author

erw7 commented May 27, 2018

@justinmk You used to say this before.

@janlazo neovim looks for terminfo in unibi_from_term as follows.

  • $TERMINFO
  • ${HOME}/.terminfo
  • $TERMINFO_DIRS
  • unibi_terminfo_dirs (Empty on Windows)

If terminfo is not found anywhere above, use builtin terminfo. In that case, since the builtin terminfo does not have cygwin, ansi_terminfo is used.

@justinmk
Copy link
Member

@justinmk You used to say this before.

Yes, that still applies :) @janlazo you may want to check those steps, to verify that Nvim found and is using the cygwin terminfo.

@janlazo
Copy link
Contributor

janlazo commented May 27, 2018

$TERMINFO, $TERMINFO_DIRS were not set and ${HOME}/.terminfo doesn't exist so it's using ansi_terminfo on my machine? I tried setting $TERMINFO to Git's /usr/share/terminfo but I can't tell the difference.

@justinmk
Copy link
Member

@janlazo Did you check the steps mentioned here: #8315 (comment)

@janlazo
Copy link
Contributor

janlazo commented May 27, 2018

@janlazo
Copy link
Contributor

janlazo commented May 27, 2018

ConEmu + bash is required for the TUI atm. Using bash on default Windows terminal doesn't display the cursor. See janlazo/dotvim8@1e2eea8

@justinmk
Copy link
Member

justinmk commented May 28, 2018

@justinmk See https://gist.github.com/janlazo/975f580d84c5f6446c09e4dd694c0d47

Ok, that confirms that Nvim isn't loading cygwin terminfo for some reason:

Oops, I didn't notice there is more than one log file.

-- Terminal info --- {{{
&term: cygwin
Description: ansi emulation for Cygwin

I wonder why unibi_from_env() doesn't find it.

@erw7
Copy link
Contributor Author

erw7 commented May 29, 2018

Looking at this log, cygwin terminfo seems to be found.

@janlazo If you add cnorm etc. in the following procedure, will the cursor be displayed?

infocmp cygwin > cygwin.ti

Added cnorm=\E[?25h,, civis=\E[?25l,, cvvis=\E[?25h, to cygwin.ti with a editor.

tic -o ${HOME}/.terminfo cygwin.ti

unset $TERMINFO.

@janlazo
Copy link
Contributor

janlazo commented May 29, 2018

After appending cygwin.ti with cnorm and etc. and running nvim -u NORC, I get a corrupted screen (not just the cursor) with bash and TERM=cygwin. On powershell with unset TERM, the TUI is still unusable.
Do I need this PR for the cursor to work?

@erw7
Copy link
Contributor Author

erw7 commented May 30, 2018

Do I need this PR for the cursor to work?

No. I think that this PR is irrelevant.

I can not imagine breaking the screen just by adding cnorm etc. Does that mean that the screen that was normal(Except that the cursor is not displayed) before the addition got strange after the addition?

Could you show me the following results(nvimlog)?

nvim -u NORC -V3nvimlog

And how about at TERM=xterm-256color(TERMINFO setting is not necessary)?

I installed Window 10 on the Virtualbox and tried it. However, the cursor will not hide under any conditions.

@janlazo
Copy link
Contributor

janlazo commented May 30, 2018

I have Windows 8.1 so I can't use TERM=xterm.

Before adding it, the screen was normal with vertical cursor (ConEmu) or block cursor (cmd.exe). I can't highlight on ConEmu for some reason. &t_Co == 8?
After adding it and creating ~/.terminfo, Vim's and Neovim's cursor broke, outputting escape sequences instead.

@erw7
Copy link
Contributor Author

erw7 commented May 31, 2018

After adding it and creating ~/.terminfo, Vim's and Neovim's cursor broke, outputting escape sequences instead.

I think you have failed to edit terminfo.

I would like to sort out a problem. Is your problem as follows?

  1. Scrolling is slow with non-default colorscheme.
  2. Using bash on default Windows terminal doesn't display the cursor.
  3. On powershell with unset TERM, the TUI is still unusable.
  4. I can't highlight on ConEmu for some reason.

@janlazo
Copy link
Contributor

janlazo commented May 31, 2018

Yes for all 4 points.

@erw7
Copy link
Contributor Author

erw7 commented Jun 1, 2018

Information is too fragmented to reproduce everything in my environment.

Are these problems caused by nvim applying this PR? Or are those problems going to happen in mintty? Otherwise, I think here is not very appropriate to continue discussion any furthrer. Would you be able to create a new issue if possible?

@justinmk
Copy link
Member

@erw7 We could merge this without finishing the TODO list, if it improve things without breaking anything. Would be nice to make the changes to tui/tui.c and ui_bridge.c somehow more isolated, if possible.

@erw7
Copy link
Contributor Author

erw7 commented May 30, 2019

@justinmk I'm sorry I can't get time, and my reply is delayed. I am skeptical about merging this PR after reading https://sourceware.org/ml/cygwin/2014-11/msg00411.html. Also, I think that the function for calling cygwin1.dll from a Windows native program is not maintained. So I do not intend to continue this work. If there is a person who needs this function, I would like that person to take over this work.

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.

5 participants