fix: long threads out of bounds
This commit is contained in:
47
tui.go
47
tui.go
@@ -1906,6 +1906,12 @@ func (m App) dashboardMaxScroll() int {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (m App) detailPaneSize() (int, int) {
|
func (m App) detailPaneSize() (int, int) {
|
||||||
|
if m.contentTop == 0 &&
|
||||||
|
len(m.difflet.frameLines()) == diffletHeight &&
|
||||||
|
!m.diffletHiddenForCurrentView() &&
|
||||||
|
m.screen != dashboardScreen {
|
||||||
|
m, _ = m.diffletContentModel()
|
||||||
|
}
|
||||||
topLines := m.threadTopLineCount()
|
topLines := m.threadTopLineCount()
|
||||||
height := max(3, m.height-topLines-1)
|
height := max(3, m.height-topLines-1)
|
||||||
if m.width < 70 || m.listHidden {
|
if m.width < 70 || m.listHidden {
|
||||||
@@ -1958,20 +1964,39 @@ func (m App) View() string {
|
|||||||
if m.screen == dashboardScreen {
|
if m.screen == dashboardScreen {
|
||||||
return m.viewDashboardWithLines(m.dashboardLinesWithDifflet(mascot))
|
return m.viewDashboardWithLines(m.dashboardLinesWithDifflet(mascot))
|
||||||
}
|
}
|
||||||
gap := diffletGap
|
content, hasHeader := m.diffletContentModel()
|
||||||
|
if !hasHeader {
|
||||||
|
return lipgloss.NewStyle().Width(m.width).Height(m.height).Render(
|
||||||
|
strings.Join(mascot, "\n") + "\n\n" + content.viewContent(),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
rendered := content.viewContent()
|
||||||
|
header, body, ok := splitHeader(rendered)
|
||||||
|
if !ok {
|
||||||
|
return lipgloss.NewStyle().Width(m.width).Height(m.height).Render(
|
||||||
|
strings.Join(mascot, "\n") + "\n\n" + rendered,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
headerBand := renderHeaderWithDifflet(
|
||||||
|
header, mascot, m.width, content.headerWidth, diffletGap,
|
||||||
|
)
|
||||||
|
return lipgloss.NewStyle().Width(m.width).Height(m.height).Render(
|
||||||
|
headerBand + "\n\n" + body,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
func (m App) diffletContentModel() (content App, hasHeader bool) {
|
||||||
headerWidth := diffletHeaderWidth(m.width)
|
headerWidth := diffletHeaderWidth(m.width)
|
||||||
if headerWidth < 1 {
|
if headerWidth < 1 {
|
||||||
headerWidth = m.width
|
headerWidth = m.width
|
||||||
}
|
}
|
||||||
content := m
|
content = m
|
||||||
content.headerWidth = headerWidth
|
content.headerWidth = headerWidth
|
||||||
headerLineCount, hasHeader := content.diffletHeaderLineCount()
|
headerLineCount, hasHeader := content.diffletHeaderLineCount()
|
||||||
if !hasHeader {
|
if !hasHeader {
|
||||||
content.height = max(1, m.height-diffletHeight-1)
|
content.height = max(1, m.height-diffletHeight-1)
|
||||||
content.contentTop = diffletHeight + 1
|
content.contentTop = diffletHeight + 1
|
||||||
return lipgloss.NewStyle().Width(m.width).Height(m.height).Render(
|
return content, hasHeader
|
||||||
strings.Join(mascot, "\n") + "\n\n" + content.viewContent(),
|
|
||||||
)
|
|
||||||
}
|
}
|
||||||
bandHeight := max(diffletHeight, headerLineCount)
|
bandHeight := max(diffletHeight, headerLineCount)
|
||||||
addedRows := bandHeight - headerLineCount
|
addedRows := bandHeight - headerLineCount
|
||||||
@@ -1980,17 +2005,7 @@ func (m App) View() string {
|
|||||||
}
|
}
|
||||||
content.height = max(1, m.height-addedRows)
|
content.height = max(1, m.height-addedRows)
|
||||||
content.contentTop = addedRows
|
content.contentTop = addedRows
|
||||||
rendered := content.viewContent()
|
return content, hasHeader
|
||||||
header, body, ok := splitHeader(rendered)
|
|
||||||
if !ok {
|
|
||||||
return lipgloss.NewStyle().Width(m.width).Height(m.height).Render(
|
|
||||||
strings.Join(mascot, "\n") + "\n\n" + rendered,
|
|
||||||
)
|
|
||||||
}
|
|
||||||
headerBand := renderHeaderWithDifflet(header, mascot, m.width, headerWidth, gap)
|
|
||||||
return lipgloss.NewStyle().Width(m.width).Height(m.height).Render(
|
|
||||||
headerBand + "\n\n" + body,
|
|
||||||
)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func (m App) diffletHiddenForCurrentView() bool {
|
func (m App) diffletHiddenForCurrentView() bool {
|
||||||
|
|||||||
55
tui_test.go
55
tui_test.go
@@ -1189,9 +1189,18 @@ func TestReplyComposerRendersInlineWithCurrentThread(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func TestLongThreadCommentRemainsReachableByScrolling(t *testing.T) {
|
func TestLongThreadCommentRemainsReachableByScrolling(t *testing.T) {
|
||||||
m := NewApp(nil, "o", "r", false, 50, time.Second)
|
for _, mascot := range []bool{false, true} {
|
||||||
m.screen, m.loading, m.width, m.height = threadScreen, false, 60, 12
|
t.Run(fmt.Sprintf("mascot=%t", mascot), func(t *testing.T) {
|
||||||
|
settings := defaultAppSettings()
|
||||||
|
settings.Mascot = mascot
|
||||||
|
m := NewAppWithSettings(
|
||||||
|
nil, "o", "r", false, 50, time.Second,
|
||||||
|
settings,
|
||||||
|
)
|
||||||
|
m.screen, m.loading = threadScreen, false
|
||||||
m.listHidden, m.focus = true, threadDetailPane
|
m.listHidden, m.focus = true, threadDetailPane
|
||||||
|
updated, _ := m.Update(tea.WindowSizeMsg{Width: 60, Height: 12})
|
||||||
|
m = updated.(App)
|
||||||
body := strings.Repeat("abcdefghij", 40) + "FINALMARKER"
|
body := strings.Repeat("abcdefghij", 40) + "FINALMARKER"
|
||||||
m.details = PRDetails{
|
m.details = PRDetails{
|
||||||
PullRequest: PullRequest{RepoWithOwner: "o/r", Number: 1, Title: "Title"},
|
PullRequest: PullRequest{RepoWithOwner: "o/r", Number: 1, Title: "Title"},
|
||||||
@@ -1208,16 +1217,54 @@ func TestLongThreadCommentRemainsReachableByScrolling(t *testing.T) {
|
|||||||
for _, line := range lines {
|
for _, line := range lines {
|
||||||
available := max(1, width-2-ansi.StringWidth(line.rail))
|
available := max(1, width-2-ansi.StringWidth(line.rail))
|
||||||
if got := ansi.StringWidth(line.fixed + line.text); got > available {
|
if got := ansi.StringWidth(line.fixed + line.text); got > available {
|
||||||
t.Fatalf("detail line width=%d available=%d text=%q", got, available, ansi.Strip(line.text))
|
t.Fatalf("detail line width=%d available=%d text=%q",
|
||||||
|
got, available, ansi.Strip(line.text))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if m.detailMaxScroll() == 0 {
|
if m.detailMaxScroll() == 0 {
|
||||||
t.Fatal("long comment did not produce scrollable detail rows")
|
t.Fatal("long comment did not produce scrollable detail rows")
|
||||||
}
|
}
|
||||||
m.scroll = m.detailMaxScroll()
|
m.scroll = m.detailMaxScroll()
|
||||||
if view := ansi.Strip(m.viewThreads()); !strings.Contains(view, "FINALMARKER") {
|
if view := ansi.Strip(m.View()); !strings.Contains(view, "FINALMARKER") {
|
||||||
t.Fatalf("final comment content is not reachable at maximum scroll:\n%s", view)
|
t.Fatalf("final comment content is not reachable at maximum scroll:\n%s", view)
|
||||||
}
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestLongThreadReplyComposerIsVisibleWithDifflet(t *testing.T) {
|
||||||
|
settings := defaultAppSettings()
|
||||||
|
settings.Mascot = true
|
||||||
|
m := NewAppWithSettings(
|
||||||
|
&recordingService{}, "o", "r", false, 50, time.Second,
|
||||||
|
settings,
|
||||||
|
)
|
||||||
|
m.screen, m.loading = threadScreen, false
|
||||||
|
m.listHidden, m.focus = true, threadDetailPane
|
||||||
|
updated, _ := m.Update(tea.WindowSizeMsg{Width: 60, Height: 12})
|
||||||
|
m = updated.(App)
|
||||||
|
m.details = PRDetails{
|
||||||
|
PullRequest: PullRequest{
|
||||||
|
ID: "pr", Owner: "o", Repository: "r",
|
||||||
|
RepoWithOwner: "o/r", Number: 1, Title: "Title",
|
||||||
|
},
|
||||||
|
Threads: []ReviewThread{{
|
||||||
|
ID: "thread", Path: "main.go", Line: 12, ViewerCanReply: true,
|
||||||
|
Comments: []ReviewComment{{
|
||||||
|
ID: "comment", Author: "reviewer",
|
||||||
|
Body: strings.Repeat("A long review comment. ", 40),
|
||||||
|
}},
|
||||||
|
}},
|
||||||
|
}
|
||||||
|
|
||||||
|
updated, _ = m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("c")})
|
||||||
|
m = updated.(App)
|
||||||
|
view := ansi.Strip(m.View())
|
||||||
|
for _, wanted := range []string{"Reply draft", "ctrl+s review"} {
|
||||||
|
if !strings.Contains(view, wanted) {
|
||||||
|
t.Fatalf("long-thread reply view is missing %q:\n%s", wanted, view)
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestResolveToggleConfirmsAndUsesCurrentThreadState(t *testing.T) {
|
func TestResolveToggleConfirmsAndUsesCurrentThreadState(t *testing.T) {
|
||||||
|
|||||||
@@ -1,3 +1,3 @@
|
|||||||
package main
|
package main
|
||||||
|
|
||||||
const dipleVersion = "0.1.2"
|
const dipleVersion = "0.1.3"
|
||||||
|
|||||||
Reference in New Issue
Block a user