feat(textarea) Add multiline placeholder

[mikelorant]

Add the capability to show a multiline placeholder. Some refactoring was required to improve readability and improve logic.

End of line buffer character was only shown when line numbers were displayed which requires some verification whether this is the intended outcome. This change resolves this issue.

[mikelorant]

@meowgorithm @maaslalani Can I get approval to run the workflow? Thanks.

[mikelorant]

Can I get someone from @charmbracelet to review this. Thanks.

[maaslalani]

Hey @mikelorant. Thank you so much for this PR.

I'm seeing a bug with how this PR is handling line numbers:

This is what the textarea should look like:

Here is what it looks like using this PR (rebased on main):

[maaslalani]

The testing code I'm using can be found here: https://github.com/charmbracelet/bubbletea/tree/master/examples/textarea

[mikelorant]

Just to clarify, line numbers only display if there is user input (or cursor) on that line?

[mikelorant]

Just did some basic investigation and it seems the aim is that it should look like Vim with line numbers enabled. Which means only lines with content get a number.

[maaslalani]

Just did some basic investigation and it seems the aim is that it should look like Vim with line numbers enabled. Which means only lines with content get a number.

Yep, that's correct! Similar to vim!

[mikelorant]

Have fixed the issue you have reported and here is the result using the code you linked:

Using a multiline placeholder shows like this:

I wish to make it clear that line numbers are only for lines that have a cursor or entered text. This is placeholder text, so only the first line will ever have a line number in this case. As soon as you start typing and adding new lines, none of this code is used.

I've had to extensively increase the unit tests because it was far too complex checking all the variations. As part of this process I needed a small helper function to strip ANSI and trailing white spaces to make comparison easier. I would have preferred to use golden files however I think this is a reasonable compromise. Unfortunately no other tests in textarea use testing tables so that will stand out as being different but as you can tell a requirement.

[mikelorant]

One more reference image with the cursor visible.

Check the test tables and let me know if that is the output you'd expect. I had to make some executive decisions and tried to base everything I could on how it was displayed in Vim.

[mikelorant]

@maaslalani Any chance you could do a review, if this is working correctly I'd like to get it merged otherwise if there are problems, lets me get started on fixing them.

[maaslalani]

Hey @mikelorant, thanks again for your work on this one!

One issue I'm seeing is the CursorLine (if it has a background color, bleeds over to the line number column)

Here's what it should look like:

[mikelorant]

The default prompt which is being used here is defined at: https://github.com/charmbracelet/bubbles/blob/master/textarea/textarea.go#L258C3-L258C59

Code as follows:

Prompt:               lipgloss.ThickBorder().Left + " ",

This has me thinking that both versions are wrong. The background colour should start at position 3 for each line.

|X123 Once upon a time...
|X    ~
|X    ~

The X space is part of that prompt and whatever styling that is applied to it is independent of everything else.

Thoughts?

Additional notes:

Consider the following change:

Prompt:               lipgloss.ThickBorder().Right + " ",

This would put the changed background colour outside the left border which would be very wrong.

[mikelorant]

@maaslalani I'll quickly do the update to this pull request providing the original functionality, but I still believe both implementations have it wrong.

Using Vim as the reference, highlighted cursor line goes as far as the beginning of the line number or end of buffer character. It is hard to know what we should do if we put a border around it since we cant do that in Vim.

[mikelorant]

Here are the two possibilities in one image:

The top version is how it is currently implemented and the bottom version is how I think it should be. However as a lowly minion here to serve the wonderful Charm team 😄 I have pushed up the version that conforms with how it is also displayed when text is entered.

Thanks for helping make sure we catch all the little subtle things, very much appreciated @maaslalani.

[mikelorant]

Heres the example showing the line style for the cursor line leaking out of the prompt "border".

Prompt to use to see this case:

ti.Prompt = lipgloss.ThickBorder().Right + " "

[maaslalani]

@mikelorant Thanks for the care you're putting into this PR! Yes, I see what you mean by the border. However, when the cursor line is defined with a background color I think it looks more correct (but maybe we will change this later since you are actually right about the correctness technically)

Also when a text area has a multiline placeholder I believe the cursor line background should be applied to the entire placeholder. In your screenshots it only applies to the first line.

[mikelorant]

Here is a comparison of the two:

I think it is very confusing to highlight more than the cursor line where there is a multiline placeholder. However, on the flip side it may help indicate that any typing will clear all that text away by keeping it "grouped".

How should I proceed?

[maaslalani]

In my opinion the second option is correct here as the entire line is highlighted with the cursor line and if the user types that exact placeholder then the same highlighting will occur.

[mikelorant]

@maaslalani Commit updated to highlight all lines of the placeholder.

[mikelorant]

@maaslalani Can you please trigger the workflow. Thanks.

[mikelorant]

@maaslalani Anything blocking this from merging? Like to get this one wrapped up.

[mikelorant]

@meowgorithm @maaslalani Any chance of getting a final review so we can get this wrapped up and merged. Thanks.

[meowgorithm]

Hey, @mikelorant! Maas is out traveling this week; I imagine he'll be able to take a look next week after he's back. He's the best person to give this one the a-ok.

[mikelorant]

@meowgorithm Thanks for the update! Will put this on the backburner then.

[mikelorant]

@maaslalani Any chance for review please?

[maaslalani]

Hey @mikelorant, will try to get this in soon. Sorry for the delay!

[mikelorant]

@maaslalani No problem, thanks for letting me know.

[maaslalani]

Noticing a bug here with single line placeholders:

The end of line buffers shift when entering a character. (The placement in the first image is correct and shouldn't shift towards the right when a character is entered.)

[maaslalani]

Also if possible can we rebase this on master to ensure it works with the latest changes of bubbles.

[mikelorant]

So I believe this is a bug with the existing textarea. Be aware I have only changed the code for the placeholder.

Here is what it looks like in Vim.

My unit tests for a placeholder is defined as the follows:

{
	name: "single line with show line numbers",
	lines: []string{
		"the first line",
	},
	showLineNumbers: true,
	expected: []string{
		">   1 the first line",
		"> ~",
		"> ~",
		"> ~",
		"> ~",
		"> ~",
	},
},

As soon as you typed a letter you were no longer using the placeholder function and the normal textarea functionality applies.

Would we like to maintain the previous (flawed) implementation or correct it to be like Vim?