Replace textarea with tui-textarea #2045

pm100 · 2024-02-13T00:54:22Z

now using tui-textarea v.04, that adds keyboard based text selection (hold down shift key while moving cursor with mouse)

reminder that a newline is made by pressing ctrl-enter or shift-enter

pm100 · 2024-02-13T01:01:06Z

still would like to point out that one of the tests fails on windows due to a privilege issue. It would be nice if that test was cfg !windows

extrawurst

a few nitpicks but ultimately the primary problem is: for me in none of the macos terminals (iterm2, Terminal, vscode embedded) the newline command works

src/components/textinput.rs

extrawurst · 2024-02-13T09:12:58Z

src/components/textinput.rs

+New line is shift-enter, ctrl-enter . These are all common new line editor commands (discord, emacs,...)
+
+There is no help for the editor window. The only thing a user really needs to know is the newline key stroke,
+but there are 10-15 ctrl key codes. I could add then to the general help popup, or make a special one for textinput


I like the idea of a specific help popup for the editor

src/components/textinput.rs

pm100 · 2024-02-13T23:16:05Z

Its odd because I partially developed on mac so this shift-enter must be new-ish problem. Anyway there is a crossterm issue that I have reported (crossterm-rs/crossterm#861). It is immediately resolvable by changing the startup code of gitui where it sets up crossterm. But that only works in some terminals (ITerm2 works, apple terminal does not).

I can also put back the ctrl-M code for newline that was there way back but one early reviewer said they didnt like it so I took it out.

I will wait a few days to see what crossterm has to say

pm100 · 2024-02-14T01:11:46Z

I have added ctrl-m back , meaing 'new line'

I have added the code to partially fix the mac issue, in my tests it works on iterm2, but not plain terminal. I dont think thats a good way to do it since its now not 100% clear to the user what will work when. The code is there so you can verify that it works.

Fixed everything else (pedantic is a pain, I think some things are less clear now, but whatever..)

Regarding adding help popup , extra key help etc... Can we get this big change in first. Everything works as before so nobody looses anything by not knowing the keystrokes

extrawurst · 2024-02-14T11:54:24Z

@pm100 I went back through #509 which contains most of the considerations around this new-line command issue. lets go back to that original plan and use a new binding like ctr+o to commit and raw enter for the newline.

pm100 · 2024-02-14T17:35:18Z

I think thats a really bad idea, people will hate it

its a breaking change
its a breaking change to the most often used path through gitui ('gitui', 'a','c','xxxxx', 'enter')
its a breaking change that is introduced solely to enable a feature that will not be used 99% of the time (multi line commit messages)
its not obvious that ctrl-o means commit. It feels like 'open' to me
its a breaking change caused by working around a bug in crossterm that only affects mac users using the native terminal

If you use my suggestion (ctrl-m for new line) then the feature is enabled, and nothing breaks, its an obscure key combination for use in a rare case. And maybe disable ctrl-enter on terminals where it doesnt work.

Its your codebase but I guarantee you will annoy lots of users. Including me, because my muscle memory will still hit enter every time (just like I hit Esc every time to close a popup dialog because that seems natural - and it does close many popups, Annoys me every time it happens). I will be annoyed but at least I know the solution, others will not know what to do.

extrawurst · 2024-02-14T17:44:11Z

i am unsure:

its been a long discussion prior. see old PR.

its not a stable version of gitui yet, it will be documented in the cmd-bar. if anything can be broken its before 1.0.

Plus: your muscle memory will lead to no breaking change, you will insert a new-line, newcomers might be put of by the fact right now that enter is not a new-line and does a commit.

plus plus: you can re-bind the command if you want to.

pm100 · 2024-02-14T19:52:36Z

How about this

I make it so that you can specify the key binding for new line (not doable at the moment )
I make it so there is only one binding for newline (there are multiple ones now, and your key config does not support that)
I add to the bottom line of the text input box text saying something like 'ctrl-n=newline'
I make the default ctrl-n (I dont like ctrl-M - its a default from tui-textarea but I override all those anyway, I think it comes from emacs)
I think the commit = enter keystroke is also configurable, if not I will make it so.

Making it hard to do the no brainer thing is a really bad idea. But with the above changes people who want to make it hard to do the 99% case and easy to do the 1% case can do exactly that. Everybody happy.

Commit = enter needs a new key list entry too (just thought about it). Enter is used all over the place - new branch name for example- I assume you want to keep 'enter' there and all the other places, NOTE, that textinput (branch name) should be set to single-line, its not , its left to default to multiline. Originally, I had fixed that (and a few others) in this PR but wanted to reduce the changes as much as possible, I got fanatical about making it one file.

So we would need a new keydef for commit_enter or something like that

pm100 · 2024-02-14T23:14:29Z

I have modified the PR so that all the changes I suggest are there, just so you can test it out and see
Only diff from above is that newline default is ctrl-r , because ctrl-n clashed

Here is the key config file for people who like it the hard way :-)

(
  multiline_enter: Some((code:Char('o') , modifiers:"CONTROL")),
  textbox_newline: Some((code:Enter, modifiers:"NONE")),
)

Includes fixing the textinput clients that should be single line (like branch name etc)

extrawurst

thanks for sticking with this. i think we are getting closer. i added a bunch of comments here and there.

extrawurst · 2024-02-15T19:57:06Z

src/components/commit.rs

@@ -565,7 +565,7 @@ impl Component for CommitComponent {
 			}

 			if let Event::Key(e) = ev {
-				if key_match(e, self.key_config.keys.enter)
+				if key_match(e, self.key_config.keys.multiline_enter)


this should be commit like we call most key bindings by its purpose elsewhere

Its a bit more fiddly than that . What should the behavior be for other multi-line edit boxes. For example 'reword commit'. I also wonder about stash name dialog, I changed it to be single-line but I think thats wrong, it can be a multi-line commit style message. Do all those behave the same way? If so (and I should check that they do work) them maybe 'commit' isnt the best name

extrawurst · 2024-02-15T20:01:50Z

src/components/create_branch.rs

@@ -104,7 +104,8 @@ impl CreateBranchComponent {
 				&strings::create_branch_popup_title(&env.key_config),
 				&strings::create_branch_popup_msg(&env.key_config),
 				true,
-			),
+			)
+			.with_input_type(InputType::Singleline),


hm good catch, this was already an oversight before but now its getting to be a very small input field. let me see if I can fix that on master separately

extrawurst · 2024-02-15T20:10:36Z

src/keys/key_list.rs

@@ -209,6 +211,8 @@ impl Default for KeysList {
 			view_submodule_parent: GituiKeyEvent::new(KeyCode::Char('p'),  KeyModifiers::empty()),
 			update_submodule: GituiKeyEvent::new(KeyCode::Char('u'),  KeyModifiers::empty()),
 			commit_history_next: GituiKeyEvent::new(KeyCode::Char('n'),  KeyModifiers::CONTROL),
+			multiline_enter: GituiKeyEvent::new(KeyCode::Enter,  KeyModifiers::empty()),
+			textbox_newline: GituiKeyEvent::new(KeyCode::Char('r'),  KeyModifiers::CONTROL),


lets call it just newline

extrawurst · 2024-02-15T20:12:25Z

src/main.rs

 	io::stdout().execute(EnterAlternateScreen)?;
 	Ok(())
 }

 fn shutdown_terminal() {
 	let leave_screen =
-		io::stdout().execute(LeaveAlternateScreen).map(|_f| ());
+		stdout().execute(LeaveAlternateScreen).map(|_f| ());


lets leave it consistent with above, this change is not needed

extrawurst · 2024-02-15T20:13:31Z

src/main.rs

+		Ok(true)
+	);
+	if supports_keyboard_enhancement {
+		crossterm::queue!(io::stdout(), PopKeyboardEnhancementFlags)


i dont know yet how to reproduce this but I get this printed after closing gitui using this branch:

PopKeyboardEnhancementFlags failed%

how can we get rid of this?

since we are not using ctrl/shift enter anymore I will remove all these changes, I dont really understand why they work, people can set it up using keyboard override if they want, but they will only do it if it works (ie not on a mac). Revisit if crossterm fixes their issue

extrawurst · 2024-02-15T20:15:37Z

src/components/textinput.rs

 	}

 	/// Get the `msg`.
+


consistency: no empty line between doc comment and fn

extrawurst · 2024-02-15T20:22:42Z

src/components/textinput.rs

-			if ends_in_nl {
-				txt.lines.push(Line::default());
-			}
+		if self.is_visible() {


why recreate the whole textarea ?

extrawurst · 2024-02-15T20:22:49Z

src/components/textinput.rs

-		self.update_count();
+		self.msg = msg.into();
+		if self.is_visible() {
+			self.create_and_show();


why recreate the whole textarea ?

Dont go there :-). This code is the result of > 6 months of work arm wrestling with tui-textarea, your API into textarea (which I took as a given) and rust lifetime management.

extrawurst · 2024-02-15T20:23:16Z

src/components/textinput.rs

-			let is_in_word = self.at_alphanumeric(index);
-			if was_in_word && !is_in_word {
-				return Some(last_pos);
+	fn create_and_show(&mut self) {


lets call it create_inner_textarea

extrawurst · 2024-02-15T20:27:12Z

src/components/textinput.rs

-				self.char_count
-			))
-			.alignment(Alignment::Right);
+	fn draw_newline_hint<B: Backend>(


lets use the machinery that is already there to feed this new command into the cmd-bar on the bottom see fn commands

i missed that, will change. And in fact that code needs to be updated to reflect the 'commit' key anyway.

pm100 · 2024-02-15T22:20:42Z

OK. we are close :-)

I did everything you said.

Notes:

Should stash msg dialog be multi or single? I made it single but I think it should be multiple. Not sure tho. If multiple I need to change the plumbing around it too.

The defaults are Enter = commit, ctrl-r = newline. I have test both these

(
commit: Some((code:Char('o') , modifiers:"CONTROL")),
newline: Some((code:Enter, modifiers:"NONE")),
)

which is the one you asked for at the start

and

(
commit: Some((code:Enter , modifiers:"NONE")),
newline: Some((code:Enter, modifiers:"SHIFT")),
)

Which is what I originally had but doesnt work on a mac - as you found out.

extrawurst requested changes Feb 13, 2024

View reviewed changes

pm100 closed this Feb 14, 2024

pm100 reopened this Feb 14, 2024

extrawurst mentioned this pull request Feb 14, 2024

Create multi-line commit messages #1171

Closed

extrawurst requested changes Feb 15, 2024

View reviewed changes

pm100 closed this Feb 15, 2024

pm100 force-pushed the tta4 branch from 9788e7d to 89ce540 Compare February 15, 2024 22:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace textarea with tui-textarea #2045

Replace textarea with tui-textarea #2045

pm100 commented Feb 13, 2024

pm100 commented Feb 13, 2024

extrawurst left a comment

extrawurst Feb 13, 2024

pm100 commented Feb 13, 2024

pm100 commented Feb 14, 2024

extrawurst commented Feb 14, 2024

pm100 commented Feb 14, 2024

extrawurst commented Feb 14, 2024

pm100 commented Feb 14, 2024 •

edited

pm100 commented Feb 14, 2024 •

edited

extrawurst left a comment

extrawurst Feb 15, 2024

pm100 Feb 15, 2024

extrawurst Feb 15, 2024

extrawurst Feb 15, 2024

extrawurst Feb 15, 2024

extrawurst Feb 15, 2024

extrawurst Feb 15, 2024

pm100 Feb 15, 2024

extrawurst Feb 15, 2024

extrawurst Feb 15, 2024

extrawurst Feb 15, 2024

pm100 Feb 15, 2024

extrawurst Feb 15, 2024

extrawurst Feb 15, 2024

pm100 Feb 15, 2024 •

edited

pm100 commented Feb 15, 2024

Replace textarea with tui-textarea #2045

Replace textarea with tui-textarea #2045

Conversation

pm100 commented Feb 13, 2024

pm100 commented Feb 13, 2024

extrawurst left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pm100 commented Feb 13, 2024

pm100 commented Feb 14, 2024

extrawurst commented Feb 14, 2024

pm100 commented Feb 14, 2024

extrawurst commented Feb 14, 2024

pm100 commented Feb 14, 2024 • edited

pm100 commented Feb 14, 2024 • edited

extrawurst left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pm100 Feb 15, 2024 • edited

Choose a reason for hiding this comment

pm100 commented Feb 15, 2024

pm100 commented Feb 14, 2024 •

edited

pm100 commented Feb 14, 2024 •

edited

pm100 Feb 15, 2024 •

edited