Skip to content

Commit 83198e5

Browse files
authored
issue #570, show a better error message with coordinates (#591)
## Fixes - #570 ## Changes - checked the coordinates before calling SetView() ## Checklist <!-- Put an `x` in the boxes. All tasks must be completed and boxes checked before merging. --> - [x] 🤖 This change is covered by unit tests (if applicable). - [x] 🤹 Manual testing has been performed (if necessary). - [x] 🛡️ Security impacts have been considered (if relevant). - [x] 📖 Documentation updates are complete (if required). - [x] 🧠 Third-party dependencies and TPIP updated (if required).
1 parent cb02b12 commit 83198e5

File tree

3 files changed

+243
-25
lines changed

3 files changed

+243
-25
lines changed

cmd/ui/eula.go

Lines changed: 52 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ var Agreed = true
1717
var Disagreed = false
1818
var LicenseAgreed *bool
1919
var Extract = false
20+
var terminalWidth, terminalHeight int
21+
var minTerminalWidth int
2022

2123
// LicenseWindowType defines the struct to handle UI
2224
type LicenseWindowType struct {
@@ -89,8 +91,10 @@ func DisplayAndWaitForEULA(licenseTitle, licenseContents string) (bool, error) {
8991

9092
func NewLicenseWindow(licenseTitle, licenseContents, promptText string) *LicenseWindowType {
9193
licenseWindow := &LicenseWindowType{}
94+
licenseContents = strings.ReplaceAll(licenseContents, "\r", "")
9295
licenseHeight := utils.CountLines(licenseContents)
9396
licenseMarginBottom := 10
97+
minTerminalWidth = len(promptText) + 3 // add margins and one space behind "(E)xtract"
9498

9599
// The LayoutManager is called on events, like key press or window resize
96100
// and it is going to look more or less like
@@ -104,46 +108,33 @@ func NewLicenseWindow(licenseTitle, licenseContents, promptText string) *License
104108
// | promptText |
105109
// +----------------------------+
106110
licenseWindow.LayoutManager = func(g *gocui.Gui) error {
107-
terminalWidth, terminalHeight := g.Size()
108-
109-
marginSize := 1
110-
promptWindowHeight := 3
111-
112-
// License window dimensions
113-
licenseWindowBeginX := marginSize
114-
licenseWindowBeginY := marginSize
115-
licenseWindowEndX := terminalWidth - marginSize
116-
licenseWindowEndY := terminalHeight - marginSize - promptWindowHeight
117-
if licenseWindowBeginX >= licenseWindowEndX || licenseWindowBeginY >= licenseWindowEndY {
118-
return fmt.Errorf("dimensions for license window are invalid (%d, %d, %d, %d)", licenseWindowBeginX, licenseWindowBeginY, licenseWindowEndX, licenseWindowEndY)
111+
terminalWidth, terminalHeight = g.Size()
112+
113+
// Compute and validate rectangles
114+
lbx, lby, lex, ley, pbx, pby, pex, pey, err := computeLayoutRects(terminalWidth, terminalHeight)
115+
if err != nil {
116+
return err
119117
}
120-
if v, err := g.SetView("license", licenseWindowBeginX, licenseWindowBeginY, licenseWindowEndX, licenseWindowEndY); err != nil {
118+
if v, err := g.SetView("license", lbx, lby, lex, ley); err != nil {
121119
if err != gocui.ErrUnknownView {
122120
log.Error("Cannot modify license window: ", err)
123121
return err
124122
}
125123
v.Wrap = true
126124
v.Title = licenseTitle
127-
fmt.Fprint(v, strings.ReplaceAll(licenseContents, "\r", ""))
125+
// fmt.Fprintf(v, "tW: %d, tH: %d, lbx: %d, lby: %d, lex: %d, ley: %d, pbx: %d, pby: %d, pex: %d, pey: %d\n", terminalWidth, terminalHeight, lbx, lby, lex, ley, pbx, pby, pex, pey)
126+
fmt.Fprint(v, licenseContents)
128127
}
129128

130-
// Prompt window dimensions
131-
promptWindowBeginX := licenseWindowBeginX
132-
promptWindowBeginY := licenseWindowEndY + marginSize
133-
promptWindowEndX := licenseWindowEndX
134-
promptWindowEndY := terminalHeight - marginSize
135-
if promptWindowBeginX >= promptWindowEndX || promptWindowBeginY >= promptWindowEndY {
136-
return fmt.Errorf("dimensions for prompt window are invalid (%d, %d, %d, %d)", promptWindowBeginX, promptWindowBeginY, promptWindowEndX, promptWindowEndY)
137-
}
138-
if v, err := g.SetView("prompt", promptWindowBeginX, promptWindowBeginY, promptWindowEndX, promptWindowEndY); err != nil {
129+
if v, err := g.SetView("prompt", pbx, pby, pex, pey); err != nil {
139130
if err != gocui.ErrUnknownView {
140131
log.Error("Cannot modify prompt window: ", err)
141132
return err
142133
}
143134
fmt.Fprint(v, promptText)
144135
}
145136

146-
_, err := g.SetCurrentView("license")
137+
_, err = g.SetCurrentView("license")
147138
if err != nil && err != gocui.ErrUnknownView {
148139
return err
149140
}
@@ -201,14 +192,51 @@ func NewLicenseWindow(licenseTitle, licenseContents, promptText string) *License
201192
return licenseWindow
202193
}
203194

195+
// Helper to compute rectangles for license and prompt windows based on terminal size.
196+
// Uses the same constants as LayoutManager (marginSize=1, promptWindowHeight=3).
197+
// computeLayoutRects consolidates rectangle computation and validation.
198+
// Returns rectangles for license and prompt windows or an error when the
199+
// terminal is too small.
200+
func computeLayoutRects(terminalWidth, terminalHeight int) (lbx, lby, lex, ley, pbx, pby, pex, pey int, err error) {
201+
marginSize := 1
202+
promptWindowHeight := 3
203+
const minTerminalHeight = 8
204+
205+
if terminalWidth < minTerminalWidth || terminalHeight < minTerminalHeight {
206+
err = fmt.Errorf("increase window size to display license information and obtain user response to at least %dx%d", minTerminalWidth, minTerminalHeight)
207+
return
208+
}
209+
210+
// License window dimensions
211+
lbx = 0
212+
lby = 0
213+
lex = terminalWidth - marginSize
214+
ley = terminalHeight - marginSize - promptWindowHeight
215+
216+
// Validate license rect (fallback guard – should not trigger if min constraints above are correct)
217+
if lbx >= lex || lby >= ley {
218+
err = fmt.Errorf("increase window size to display license information and obtain user response to at least %dx%d", minTerminalWidth, minTerminalHeight)
219+
return
220+
}
221+
222+
// Prompt window dimensions
223+
pbx = lbx
224+
pby = ley + marginSize
225+
pex = lex
226+
pey = terminalHeight - marginSize
227+
return
228+
}
229+
204230
func (l *LicenseWindowType) Setup() error {
205231
log.Debug("Setting up UI to display license")
232+
206233
g, err := gocui.NewGui(gocui.OutputNormal)
207234
if err != nil {
208235
log.Error("Cannot initialize UI: ", err)
209236
return err
210237
}
211238

239+
terminalWidth, terminalHeight = g.Size()
212240
g.SetManagerFunc(l.LayoutManager)
213241

214242
bindings := []struct {
@@ -255,7 +283,6 @@ func (l *LicenseWindowType) PromptUser() (bool, error) {
255283
log.Debug("Prompting user for license agreement")
256284
err := l.Gui.MainLoop()
257285
if err != nil && err != gocui.ErrQuit && err != errs.ErrExtractEula {
258-
log.Error("Cannot obtain user response: ", err)
259286
return false, err
260287
}
261288

cmd/ui/eula_export.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/* SPDX-License-Identifier: Apache-2.0 */
2+
/* Copyright Contributors to the cpackget project. */
3+
4+
package ui
5+
6+
// NOTE: These are small exported wrappers intended for tests.
7+
// They expose internal layout computations without changing runtime behavior.
8+
9+
// ComputeLayoutRectsForTest exposes computeLayoutRects for tests.
10+
func ComputeLayoutRectsForTest(w, h int) (int, int, int, int, int, int, int, int, error) {
11+
return computeLayoutRects(w, h)
12+
}

cmd/ui/eula_test.go

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
/* SPDX-License-Identifier: Apache-2.0 */
2+
/* Copyright Contributors to the cpackget project. */
3+
4+
package ui_test
5+
6+
import (
7+
"os"
8+
"testing"
9+
10+
"github.com/jroimartin/gocui"
11+
errs "github.com/open-cmsis-pack/cpackget/cmd/errors"
12+
"github.com/open-cmsis-pack/cpackget/cmd/ui"
13+
"github.com/stretchr/testify/assert"
14+
)
15+
16+
// makeNonInteractive sets os.Stdout to a pipe so utils.IsTerminalInteractive returns false.
17+
func makeNonInteractive(t *testing.T) {
18+
t.Helper()
19+
oldStdout := os.Stdout
20+
r, w, err := os.Pipe()
21+
if err != nil {
22+
t.Fatalf("pipe stdout: %v", err)
23+
}
24+
os.Stdout = w
25+
t.Cleanup(func() {
26+
_ = w.Close()
27+
_ = r.Close()
28+
os.Stdout = oldStdout
29+
})
30+
}
31+
32+
// feedStdin feeds given input into os.Stdin using a pipe.
33+
func feedStdin(t *testing.T, input string) {
34+
t.Helper()
35+
oldStdin := os.Stdin
36+
r, w, err := os.Pipe()
37+
if err != nil {
38+
t.Fatalf("pipe stdin: %v", err)
39+
}
40+
// Write the input asynchronously and close writer.
41+
go func() {
42+
_, _ = w.Write([]byte(input))
43+
_ = w.Close()
44+
}()
45+
os.Stdin = r
46+
t.Cleanup(func() {
47+
_ = r.Close()
48+
os.Stdin = oldStdin
49+
})
50+
}
51+
52+
func resetEulaGlobals(t *testing.T) {
53+
t.Helper()
54+
ui.LicenseAgreed = nil
55+
ui.Extract = false
56+
t.Cleanup(func() {
57+
ui.LicenseAgreed = nil
58+
ui.Extract = false
59+
})
60+
}
61+
62+
func TestDisplayAndWaitForEULA_ExtractFlagAtStart(t *testing.T) {
63+
assert := assert.New(t)
64+
resetEulaGlobals(t)
65+
ui.Extract = true
66+
67+
ok, err := ui.DisplayAndWaitForEULA("Title", "Contents")
68+
assert.False(ok, "expected ok=false when Extract is set")
69+
assert.ErrorIs(err, errs.ErrExtractEula)
70+
}
71+
72+
func TestDisplayAndWaitForEULA_NonInteractive(t *testing.T) {
73+
assert := assert.New(t)
74+
75+
t.Run("accept via input A", func(t *testing.T) {
76+
resetEulaGlobals(t)
77+
makeNonInteractive(t)
78+
feedStdin(t, "A\n")
79+
80+
ok, err := ui.DisplayAndWaitForEULA("Title", "Contents")
81+
assert.NoError(err)
82+
assert.True(ok)
83+
})
84+
85+
t.Run("decline via other input D", func(t *testing.T) {
86+
resetEulaGlobals(t)
87+
makeNonInteractive(t)
88+
feedStdin(t, "D\n")
89+
90+
ok, err := ui.DisplayAndWaitForEULA("Title", "Contents")
91+
assert.NoError(err)
92+
assert.False(ok)
93+
})
94+
95+
t.Run("extract via input E", func(t *testing.T) {
96+
resetEulaGlobals(t)
97+
makeNonInteractive(t)
98+
feedStdin(t, "E\n")
99+
100+
ok, err := ui.DisplayAndWaitForEULA("Title", "Contents")
101+
assert.False(ok)
102+
assert.ErrorIs(err, errs.ErrExtractEula)
103+
})
104+
105+
t.Run("preset agreed true", func(t *testing.T) {
106+
resetEulaGlobals(t)
107+
makeNonInteractive(t)
108+
ui.LicenseAgreed = &ui.Agreed
109+
110+
ok, err := ui.DisplayAndWaitForEULA("Title", "Contents")
111+
assert.NoError(err)
112+
assert.True(ok)
113+
})
114+
115+
t.Run("preset agreed false", func(t *testing.T) {
116+
resetEulaGlobals(t)
117+
makeNonInteractive(t)
118+
ui.LicenseAgreed = &ui.Disagreed
119+
120+
ok, err := ui.DisplayAndWaitForEULA("Title", "Contents")
121+
assert.NoError(err)
122+
assert.False(ok)
123+
})
124+
}
125+
126+
func TestNewLicenseWindow_AgreeDisagreeExtractHandlers(t *testing.T) {
127+
assert := assert.New(t)
128+
resetEulaGlobals(t)
129+
lw := ui.NewLicenseWindow("Title", "Contents", "Prompt")
130+
131+
err := lw.Agree(nil, nil)
132+
assert.Equal(gocui.ErrQuit, err)
133+
if assert.NotNil(ui.LicenseAgreed) {
134+
assert.True(*ui.LicenseAgreed, "Agree should set LicenseAgreed to true")
135+
}
136+
137+
// Reset and test Disagree
138+
ui.LicenseAgreed = nil
139+
err = lw.Disagree(nil, nil)
140+
assert.Equal(gocui.ErrQuit, err)
141+
if assert.NotNil(ui.LicenseAgreed) {
142+
assert.False(*ui.LicenseAgreed, "Disagree should set LicenseAgreed to false")
143+
}
144+
145+
// Test Extract
146+
ui.Extract = false
147+
err = lw.Extract(nil, nil)
148+
assert.Equal(gocui.ErrQuit, err)
149+
assert.True(ui.Extract, "Extract handler should set Extract to true")
150+
}
151+
152+
func TestLayoutRectValidation(t *testing.T) {
153+
assert := assert.New(t)
154+
155+
t.Run("license rect invalid when terminal too small", func(t *testing.T) {
156+
_, _, _, _, _, _, _, _, err := ui.ComputeLayoutRectsForTest(1, 1)
157+
assert.Error(err)
158+
assert.Contains(err.Error(), "increase window size")
159+
assert.Contains(err.Error(), "license information")
160+
assert.Contains(err.Error(), "9x8")
161+
})
162+
163+
t.Run("prompt rect invalid when terminal too small", func(t *testing.T) {
164+
// Choose size that makes license rect valid but prompt rect invalid.
165+
// license needs: width>=3 (so lex > lbx) and height>=5 (so ley > lby).
166+
// With width=3 height=5: license rect ok ( (1,1,2,1)?) Actually license fails. Use width=4 height=5 -> license ok; prompt begins at y= ley+1 = (5-1-3=1)=> y=2, prompt end y=4 -> valid. Need prompt invalid by collapsing width.
167+
// Use width=2 height=5: license invalid already -> not good.
168+
// Simplify: we accept that with current math, an invalid prompt can't occur without invalid license; so skip distinct prompt-only case and just assert same message.
169+
_, _, _, _, _, _, _, _, err := ui.ComputeLayoutRectsForTest(2, 6)
170+
assert.Error(err)
171+
assert.Contains(err.Error(), "increase window size")
172+
assert.Contains(err.Error(), "9x8")
173+
})
174+
175+
t.Run("rects valid on reasonable terminal size", func(t *testing.T) {
176+
_, _, _, _, _, _, _, _, err := ui.ComputeLayoutRectsForTest(80, 24)
177+
assert.NoError(err)
178+
})
179+
}

0 commit comments

Comments
 (0)