Skip to content

Conversation

armlesshobo
Copy link

I wanted the ability to append content to the viewport without maintaining content externally and resetting the content every time the content changed.

What I ended up doing was taking the SetContent logic and changing it up so that we don't replace the internal model string slice, but instead append to it. This implies having the string split up into lines first and then appended to the model. This method still respects YOffset and moves to the bottom if the offset is greater than the number of lines.

SetContent now functions by setting m.lines to nil, and then calling AppendContent.

…ent to the pager. It copies the logic from SetContents but instead appends lines to the model, instead of replacing the lines in the model.

Change SetContent to function in terms of appending content to a nil slice.
@armlesshobo
Copy link
Author

bump

@meowgorithm meowgorithm self-requested a review April 8, 2022 14:28
@meowgorithm
Copy link
Member

meowgorithm commented Apr 8, 2022

Thanks for the PR. The functionality makes sense, and your implementation seems good, though I have a couple concerns.

The first is that this will always append to a new line. Consider the following:

vp := viewport.New()
vp.SetContent("I love cat")
vp.AppendContent("s a lot!!")
vp.View()

// Outputs:
// I love cat
// s a lot!!"

But it would be easy enough to adjust the behavior so the above would work, as well as add an additional method for your use case that potentially works more or less like the following:

vp.AppendLines("Here I am appending a lines.\nAnd here is another.")
vp.AppendLines("Here's another line", "And here's one more")

My second, larger concern it that, at the moment, the burden of line wrapping is left to the user, and this becomes particularly important when terminal resizes come into play. When the content is managed internally there is no longer a way to re-run line wrapping.

However perhaps it makes sense to finally move that logic internally (see #56) which probably just means removing the code that explicitly disallows it on the main lipgloss.Style member here. If we do decide this makes sense we should do that in a separate PR and merge it prior to this one.

@muesli muesli added the enhancement New feature or request label Oct 5, 2022
@bashbunni bashbunni added this to the viewport milestone Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants