fix cursor lost at end of line #395
Conversation
|
I guess these double bars is what you meant? |
|
on my system it doesnt look that bad (different fonts I guess) I will change it so that the trailing space is not underlined. Give me a sec As I said the true solution needs a tui fix |
|
now testing this I realised we have a bigger issue:
new lines are not applied anymore. I suspect this is related to out tui-rs upgrade #289 by @WizardOhio24 - can you check if we can fix this in this PR aswell? |
|
You are most likely correct. With the TUI update, each new line needs to be in a separate Spans, text in the same Spans is rendered on the same line, and so this code renders all characters on the same line without regard for type of character. https://github.com/extrawurst/gitui/blob/c4fdbf7aba9b6925b941816e84f4459746cbeb89/src/components/textinput.rs#L134-L138 |
|
The failure happens when there is only one line, it is for sure a tui bug. Look at the screen shots I posted in the tui bug report
ehm no, the screenshot i posted is actually two lines
…On Sun, Nov 1, 2020 at 6:57 PM pm100 ***@***.***> wrote:
You are most likely correct. With the TUI update, each new line needs to
be in a separate Spans, text in the same Spans is rendered on the same
line, and so this code renders all characters on the same line without
regard for type of character.
https://github.com/extrawurst/gitui/blob/c4fdbf7aba9b6925b941816e84f4459746cbeb89/src/components/textinput.rs#L134-L138
To fix this you just need to split the lines by '\n', inserting a new
spans where each is found(just split, map). I did this somewhere when I was
updating TUI I think. I believe there was a TUI issue to have text be able
to display '\n'(i.e insert a newline) if it wasn't in spans but I'm not
sure how it was resolved.
The failure happens when there is only one line, it is for sure a tui bug.
Look at the screen shots I posted in the tui bug report
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#395 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAF5U4AAPUEU5MMFIHCUAGLSNWORBANCNFSM4TGEO4CA>
.
|
|
interestingly enough using the external editor on windows (I tried 2 different ones) ends up ignoring the new lines completely.
|
|
That’s exactly the problem for which wizard described the cause
…On Sun 1. Nov 2020 at 19:49, pm100 ***@***.***> wrote:
interestingly enough using the external editor on windows (I tried 2
different ones) ends up ignoring the new lines completely.
[image: image]
<https://user-images.githubusercontent.com/489231/97811503-de16aa00-1c2f-11eb-92c4-36f3be7706db.png>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#395 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAF5U4GJB62Y3ZJOEJ3WVYDSNWUTXANCNFSM4TGEO4CA>
.
|
|
you want me to roll that fix into this PR? |
|
we can actually tackle this in another PR first if you like to split it up, but this one has to wait then until we have the general line breaking issue is fixed |
|
I am happy to fix it, just wanted to be clear |
|
A big change. I effectively implemented a mini text editor in the popup. Not finished but please build it and check it out to see if you think the approach is OK. Much more testing and some clean up needed. NOTE - I added ^Enter to add a line break (in a line or at the end) I need to add this to the status line key hints. |
|
@pm100 ok we have a misunderstanding here :( |
|
So I should make the commit msg read only when you return from ^e? BTW: The ^Enter idea came from here , I found it when I was trying to work out how to make ^e work. |
|
Don't know if this helps now, but I found the text PR in TUI: fdehau/tui-rs#361, which allows for text to display "\n"(i.e insert a new line). It might be easier to use what was introduced there, as in allow newline characters, then there doesn't really need to be much change to the way it's currently implemented and should remove the read-only issue. |
|
it supported what? The current commit box doesnt support multiple lines, It has no code for up or down arrow, it has no concept of current line. Tell me what you want the behavior to be and I will make it so this PR supports multiple lines, up / down arrows, scroll if not enough space in the screen to show all lines. I also added the ability to spilt and unsplit lines Tell me what features you want in or out an I will do it |
|
to be clear - this is not a display issue, thats trivial 2 min fix. The issue is - should the user be allowed to edit a multi line commit message? If 'no' then I will make the commit box read only. If 'yes' then we need this PR. Also what should happen if the multi line text has more lines than can be displayed, which lines should be displayed? I will point out that users will be surprised that they can no longer edit when the return from ^e |
|
So, I think what @extrawurst is saying is that this PR was only to fix the missing cursor at the end of the line. A multiline editor(A great new feature so the user doesn't have to use an external editor), is a different PR. So, I think the easiest thing to do is to branch from "Implement full multi-line support in textinput" commit(225b972) and put it in a different PR. (Correct me if I'm wrong) I checked out 10.1 and in it you could modify multiple lines but you could only do it by moving right, so you had to keep going right until it "wrapped round" onto a new line. So I think the way you've implemented it is probably better/easier for the user, i.e using the up/down arrow keys. |
|
i will close this PR. I will resubmit the original PR for this bug I will open a separate issue for the multi line issues |
No description provided.