Add new line support for multi-line input box (follow up PR) #1309
Conversation
|
@guzzit thanks for your interest in picking this up. will you bring it up to date with master please? |
fix command stack add to changelog
guzzit
force-pushed
the
multi-line-commit
branch
from
ce2e584 to
4795b43
Compare
|
Done @extrawurst |
src/components/textinput.rs
Outdated
| if i >= self.cursor_position { | ||
| if i == self.cursor_position { | ||
| //for when cursor position is on a new line just before text | ||
| if c == '\n' { |
There was a problem hiding this comment.
like in the previous PR I am not sure this will work on windows where line breaks are two bytes AFAIK?
| @@ -19,6 +19,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
| ### Fixes | |||
| * remove insecure dependency `ansi_term` ([#1290](https://github.com/extrawurst/gitui/issues/1290)) | |||
|
|
|||
| ### Changed | |||
| * `enter` adds *newline* to commit msg (*commit* via `ctrl+o` now) [[@WizardOhio24](https://github.com/WizardOhio24)] ([#509](https://github.com/extrawurst/gitui/issues/509)) | |||
There was a problem hiding this comment.
do not forget to add yourself here
| @@ -43,6 +44,10 @@ pub struct TextInputComponent { | |||
| input_type: InputType, | |||
| current_area: Cell<Rect>, | |||
| embed: bool, | |||
| scroll_top: usize, // The current scroll from the top | |||
There was a problem hiding this comment.
this can be simplified, using VerticalScroll. please see usage of VerticalScroll in other places
| top_line_start = middle_line_start; | ||
| top_line_end = middle_line_end; | ||
| middle_line_start = bottom_line_start; | ||
| middle_line_end = i.saturating_sub(1); |
There was a problem hiding this comment.
this does not look like it will work on multibyte chars. we need to unittest this function
|
|
||
| /// Move the cursor down a line. | ||
| /// Only for multi-line textinputs | ||
| fn line_down_cursor(&mut self) { |
There was a problem hiding this comment.
lots of code duplication with the above function. can this be combined?
| @@ -130,9 +322,27 @@ impl TextInputComponent { | |||
| Some(index) | |||
| } | |||
|
|
|||
| /// Backspace for multiline textinputs | |||
| fn multiline_backspace(&mut self) { | |||
There was a problem hiding this comment.
this guy needs unittesting aswell
| self.get_msg(0..self.cursor_position); | ||
| let text_before_cursor: String = self | ||
| .get_msg(0..self.cursor_position) | ||
| .split('\n') |
There was a problem hiding this comment.
again not sure this will work on all platforms where lineendings might be two bytes
|
putting in draft until the required changes are made |
|
Was this idea abandoned? Tracking this feature request across different tickets leads to this final leaf which seems to be dormant at this time. |
|
Hi @al3xandru, I'm not working on it at the moment. Please feel free to pick up the issue if you're interested. |
This Pull Request completes #509.
I followed the checklist:
make checkwithout errors (I will when I rebase with master)