[wolfwfr]

Closes #1038.

Description

This change prevents the width of the output of help.FullHelpView() & help.ShortHelpView() from exceeding the configured 'help.width' when help.width > 0, even when help.width allows enough space for the first x items, but not enough for a tailing truncation-symbol (space+ellipsis).

Prior to this change, the functions would return an entirely untruncated response that would exceed the configured help.width, in the herein described scenario.

Technical Explanation

The pre-existing logic for assembling the string of help-items, would loop through the list of items, without awareness of whether another item would later be added too. When the configured help.width would be sufficient for the current item, the item would be added. When the configured help.width would not be sufficient, the logic would attempt to add a tail instead, consisting of a space & ellipsis (by default) with a total width of 2. If the remaining space was not sufficient for this tail, the logic would instead allow the addition of the item, and the next, and so forth.

In effect, when a subset of help items would fit within help.width with a margin of 0-2, the functions would never truncate, and instead return all items, thus exceeding the configured help.width.

Fix

In order to fix this, I adjusted the loop to be context-aware, i.e. it only appends a help-item when it knows whether or not the item is going to be succeeded by another or not. If it is, the logic ensures there is enough space for BOTH the item & the tail. If the item is not succeeded, it removes the requirement for space for the tail.

The tests were expanded to ensure that the correct response is returned for all edge-cases.

When help.width == 0, all items are returned. While this technically violates the configured help.width, I did not alter this behaviour, because it sat behind a dedicated check and could make sense in use-cases where width is not explicitly configured at all.

AI Notice

In case it matters, no form of AI was involved in the design, implementation, or any other facet of this change.

• I have read CONTRIBUTING.md.
• I have created a discussion that was approved by a maintainer (for new features).

DEMO

Short Help View

BUG

FIX

Full Help View

BUG

FIX

[MohammediYunus]

I found one regression with a custom Ellipsis at 878f9c5: both shortcuts disappear even when every item fits.

With Ellipsis set to "... more", the two bindings below fit in 9 columns for ShortHelpView and 10 for FullHelpView. Base 8cea431 displays both bindings at those widths; this PR returns only a space followed by "... more".

The new loop reserves space for the tail before measuring the final item or column. If that tail is wider than the remaining content, it returns early. Could the fully fitting case be preserved before reserving tail space?

Here is a regression test that passes on the base and fails both cases on this head. The existing help tests pass on both versions.

package help

import (
	"testing"

	"charm.land/bubbles/v2/key"
	"github.com/charmbracelet/x/ansi"
)

func TestHelpCustomEllipsisDoesNotHideFittingItems(t *testing.T) {
	bindings := []key.Binding{
		key.NewBinding(key.WithKeys("a"), key.WithHelp("a", "b")),
		key.NewBinding(key.WithKeys("c"), key.WithHelp("c", "d")),
	}
	for _, tc := range []struct {
		name  string
		width int
		want  string
	}{
		{"short", 9, "a b • c d"},
		{"full", 10, "a b    c d"},
	} {
		t.Run(tc.name, func(t *testing.T) {
			m := New()
			m.Ellipsis = "... more"
			m.SetWidth(tc.width)
			got := m.ShortHelpView(bindings)
			if tc.name == "full" {
				got = m.FullHelpView([][]key.Binding{{bindings[0]}, {bindings[1]}})
			}
			if got = ansi.Strip(got); got != tc.want {
				t.Fatalf("got %q, want %q", got, tc.want)
			}
		})
	}
}

[wolfwfr]

He, thanks for the feedback and that's a good catch! I can reproduce it and see the problem. I'm considering a few different avenues for the fix and will explore them when I have a little bit more time.

I'll try to provide an update shortly.

[MohammediYunus]

Thanks for the update! I reran the original cases at 786d49b. With Ellipsis set to "... more", both bindings now remain visible at 9 columns in ShortHelpView and 10 in FullHelpView.

The help package tests pass. The new short/full TestWideEllipsisCompatibility cases also fail against the previous implementation and pass with this update.

I reran the same 48-case renderer check with standard/custom ellipses and disabled entries. Fully fitting content stays visible, and no bounded output exceeds its configured width. This resolves the regression I reported. These checks cover renderer strings and cell widths, not interactive terminal behavior.

[wolfwfr]

I pushed an update that resolves the issue raised.

I reworked the logic to the point where the PR title is no longer appropriate, the 'context-awareness' has been replaced by a mechanism that primarily attempts to fit all items and snapshots a subset that leaves enough room for the configured ellipsis in case the primary attempt fails (i.e. the model width boundary is exceeded).

NOTE 1

Do note that I did not change the behaviour where printing of the ellipsis takes precedence over printing the items, if not all items fit within the width. This is intentional (on my part), but I'm raising it here in case others disagree. This means that if a small item, e.g. a b fits within the width, but the ellipsis (e.g. ... more) alone (i.e. ellipsis with no items at all) does not, then the view returns nothing, because when the width boundary is exceeded by the items, it prioritises printing the ellipsis over any items that may have fit individually.

More concretely, with 8-char wide ellipsis .. more:

items	width	output
a b, c d	< 8
a b, c d	8	.. more
a b	> 8	a b • c d

I deemed this an edge-case that is not worth the dedicated complexity, but if anyone disagrees, I'll look into it. I think that it would only mean a small departure from the current logic.

NOTE 2

Also note that I included the test brought up in the review, but adjusted it to use the golden package, in an attempt to bring it more in line with the rest of the bubbles library.

--- edit ---

@MohammediYunus, wow you're fast, that's appreciated, thanks!