Skip to content

Commit 7ac4434

Browse files
steipetesteipete
committed
fix: harden sheets notes output (#208) (thanks @andybergon)
1 parent f00ef83 commit 7ac4434

3 files changed

Lines changed: 180 additions & 22 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
- Contacts: update contacts from JSON via `contacts update --from-file` (PR #200 — thanks @jrossi).
99
- Drive: add `drive ls|search --no-all-drives` to restrict queries to "My Drive" for faster/narrower results. (#258)
1010
- Gmail: add `gmail send --quote` to include quoted original message in replies. (#169) — thanks @terry-li-hm.
11+
- Sheets: add `sheets notes` to read cell notes. (#208) — thanks @andybergon.
1112

1213
### Fixed
1314
- Auth: manual OAuth flow uses an ephemeral loopback redirect port (avoids unsafe/privileged ports in browsers). (#172) — thanks @spookyuser.

‎internal/cmd/sheets_notes.go‎

Lines changed: 93 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ import (
44
"context"
55
"fmt"
66
"os"
7+
"regexp"
78
"strings"
8-
"text/tabwriter"
99

1010
"github.com/steipete/gogcli/internal/outfmt"
1111
"github.com/steipete/gogcli/internal/ui"
@@ -23,7 +23,7 @@ func (c *SheetsNotesCmd) Run(ctx context.Context, flags *RootFlags) error {
2323
return err
2424
}
2525

26-
spreadsheetID := strings.TrimSpace(c.SpreadsheetID)
26+
spreadsheetID := normalizeGoogleID(strings.TrimSpace(c.SpreadsheetID))
2727
rangeSpec := cleanRange(c.Range)
2828
if spreadsheetID == "" {
2929
return usage("empty spreadsheetId")
@@ -39,13 +39,16 @@ func (c *SheetsNotesCmd) Run(ctx context.Context, flags *RootFlags) error {
3939

4040
resp, err := svc.Spreadsheets.Get(spreadsheetID).
4141
Ranges(rangeSpec).
42-
Fields("sheets(data(rowData(values(note,formattedValue))))").
42+
IncludeGridData(true).
43+
Fields("sheets(properties(title),data(startRow,startColumn,rowData(values(note,formattedValue))))").
4344
Do()
4445
if err != nil {
4546
return err
4647
}
4748

4849
type cellNote struct {
50+
Sheet string `json:"sheet"`
51+
A1 string `json:"a1"`
4952
Row int `json:"row"`
5053
Col int `json:"col"`
5154
Value string `json:"value"`
@@ -55,15 +58,37 @@ func (c *SheetsNotesCmd) Run(ctx context.Context, flags *RootFlags) error {
5558
var notes []cellNote
5659

5760
for _, sheet := range resp.Sheets {
61+
if sheet == nil {
62+
continue
63+
}
64+
sheetTitle := ""
65+
if sheet.Properties != nil {
66+
sheetTitle = strings.TrimSpace(sheet.Properties.Title)
67+
}
5868
for _, data := range sheet.Data {
69+
if data == nil {
70+
continue
71+
}
72+
startRow := int(data.StartRow)
73+
startCol := int(data.StartColumn)
5974
for ri, row := range data.RowData {
75+
if row == nil {
76+
continue
77+
}
6078
for ci, cell := range row.Values {
79+
if cell == nil {
80+
continue
81+
}
6182
if cell.Note == "" {
6283
continue
6384
}
85+
absRow := startRow + ri + 1
86+
absCol := startCol + ci + 1
6487
notes = append(notes, cellNote{
65-
Row: ri + 1,
66-
Col: ci + 1,
88+
Sheet: sheetTitle,
89+
A1: formatA1Cell(sheetTitle, absRow, absCol),
90+
Row: absRow,
91+
Col: absCol,
6792
Value: cell.FormattedValue,
6893
Note: cell.Note,
6994
})
@@ -73,9 +98,10 @@ func (c *SheetsNotesCmd) Run(ctx context.Context, flags *RootFlags) error {
7398
}
7499

75100
if outfmt.IsJSON(ctx) {
76-
return outfmt.WriteJSON(os.Stdout, map[string]any{
77-
"range": rangeSpec,
78-
"notes": notes,
101+
return outfmt.WriteJSON(ctx, os.Stdout, map[string]any{
102+
"spreadsheetId": spreadsheetID,
103+
"range": rangeSpec,
104+
"notes": notes,
79105
})
80106
}
81107

@@ -84,12 +110,66 @@ func (c *SheetsNotesCmd) Run(ctx context.Context, flags *RootFlags) error {
84110
return nil
85111
}
86112

87-
tw := tabwriter.NewWriter(os.Stdout, 0, 4, 2, ' ', 0)
88-
fmt.Fprintln(tw, "ROW\tCOL\tVALUE\tNOTE")
113+
w, flush := tableWriter(ctx)
114+
defer flush()
115+
fmt.Fprintln(w, "A1\tVALUE\tNOTE")
89116
for _, n := range notes {
90-
noteLine := strings.ReplaceAll(n.Note, "\n", "\\n")
91-
fmt.Fprintf(tw, "%d\t%d\t%s\t%s\n", n.Row, n.Col, n.Value, noteLine)
117+
fmt.Fprintf(w, "%s\t%s\t%s\n",
118+
oneLine(n.A1),
119+
oneLine(n.Value),
120+
oneLine(n.Note),
121+
)
92122
}
93-
_ = tw.Flush()
94123
return nil
95124
}
125+
126+
var simpleSheetNameRe = regexp.MustCompile(`^[A-Za-z0-9_]+$`)
127+
128+
func formatA1Cell(sheetTitle string, row, col int) string {
129+
colLetters, err := colIndexToLetters(col)
130+
if err != nil || row <= 0 {
131+
return ""
132+
}
133+
cell := fmt.Sprintf("%s%d", colLetters, row)
134+
if strings.TrimSpace(sheetTitle) == "" {
135+
return cell
136+
}
137+
return formatSheetPrefix(sheetTitle) + cell
138+
}
139+
140+
func formatSheetPrefix(sheetTitle string) string {
141+
title := strings.TrimSpace(sheetTitle)
142+
if title == "" {
143+
return ""
144+
}
145+
if simpleSheetNameRe.MatchString(title) {
146+
return title + "!"
147+
}
148+
escaped := strings.ReplaceAll(title, "'", "''")
149+
return "'" + escaped + "'!"
150+
}
151+
152+
func colIndexToLetters(col int) (string, error) {
153+
if col <= 0 {
154+
return "", fmt.Errorf("invalid column index %d", col)
155+
}
156+
var b []byte
157+
for col > 0 {
158+
col--
159+
b = append(b, byte('A'+(col%26)))
160+
col /= 26
161+
}
162+
for i, j := 0, len(b)-1; i < j; i, j = i+1, j-1 {
163+
b[i], b[j] = b[j], b[i]
164+
}
165+
return string(b), nil
166+
}
167+
168+
func oneLine(s string) string {
169+
s = strings.ReplaceAll(s, "\r\n", "\n")
170+
s = strings.ReplaceAll(s, "\r", "\n")
171+
// Keep output parseable in tables/TSV.
172+
s = strings.ReplaceAll(s, "\t", " ")
173+
s = strings.ReplaceAll(s, "\n", "\\n")
174+
return s
175+
}

‎internal/cmd/sheets_notes_test.go‎

Lines changed: 86 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,28 @@ func notesHandler() http.Handler {
2222
path := strings.TrimPrefix(r.URL.Path, "/sheets/v4")
2323
path = strings.TrimPrefix(path, "/v4")
2424
if strings.HasPrefix(path, "/spreadsheets/s1") && r.Method == http.MethodGet {
25+
if r.URL.Query().Get("includeGridData") != "true" {
26+
http.Error(w, "expected includeGridData=true", http.StatusBadRequest)
27+
return
28+
}
29+
30+
rangeParam := r.URL.Query().Get("ranges")
31+
startRow, startCol := 0, 0
32+
if strings.Contains(rangeParam, "B2") {
33+
startRow, startCol = 1, 1
34+
}
35+
2536
w.Header().Set("Content-Type", "application/json")
2637
_ = json.NewEncoder(w).Encode(map[string]any{
2738
"sheets": []map[string]any{
2839
{
40+
"properties": map[string]any{
41+
"title": "Sheet1",
42+
},
2943
"data": []map[string]any{
3044
{
45+
"startRow": startRow,
46+
"startColumn": startCol,
3147
"rowData": []map[string]any{
3248
{
3349
"values": []map[string]any{
@@ -105,6 +121,18 @@ func TestSheetsNotesCmd_JSON(t *testing.T) {
105121
}
106122

107123
first := notes[0].(map[string]any)
124+
if first["sheet"] != "Sheet1" {
125+
t.Errorf("expected sheet 'Sheet1', got %q", first["sheet"])
126+
}
127+
if first["a1"] != "Sheet1!A1" {
128+
t.Errorf("expected a1 'Sheet1!A1', got %q", first["a1"])
129+
}
130+
if first["row"] != float64(1) {
131+
t.Errorf("expected row 1, got %v", first["row"])
132+
}
133+
if first["col"] != float64(1) {
134+
t.Errorf("expected col 1, got %v", first["col"])
135+
}
108136
if first["note"] != "Header note" {
109137
t.Errorf("expected 'Header note', got %q", first["note"])
110138
}
@@ -150,11 +178,60 @@ func TestSheetsNotesCmd_Text(t *testing.T) {
150178
if !strings.Contains(out, "Estimated") {
151179
t.Errorf("expected 'Estimated' in output: %q", out)
152180
}
153-
if !strings.Contains(out, "ROW") {
181+
if !strings.Contains(out, "A1") {
154182
t.Errorf("expected table header in output: %q", out)
155183
}
156184
}
157185

186+
func TestSheetsNotesCmd_OffsetRange_JSON(t *testing.T) {
187+
origNew := newSheetsService
188+
t.Cleanup(func() { newSheetsService = origNew })
189+
190+
srv := httptest.NewServer(notesHandler())
191+
defer srv.Close()
192+
193+
svc, err := sheets.NewService(context.Background(),
194+
option.WithoutAuthentication(),
195+
option.WithHTTPClient(srv.Client()),
196+
option.WithEndpoint(srv.URL+"/"),
197+
)
198+
if err != nil {
199+
t.Fatalf("NewService: %v", err)
200+
}
201+
newSheetsService = func(context.Context, string) (*sheets.Service, error) { return svc, nil }
202+
203+
flags := &RootFlags{Account: "a@b.com"}
204+
u, uiErr := ui.New(ui.Options{Stdout: io.Discard, Stderr: io.Discard, Color: "never"})
205+
if uiErr != nil {
206+
t.Fatalf("ui.New: %v", uiErr)
207+
}
208+
ctx := ui.WithUI(context.Background(), u)
209+
ctx = outfmt.WithMode(ctx, outfmt.Mode{JSON: true})
210+
211+
out := captureStdout(t, func() {
212+
if err := runKong(t, &SheetsNotesCmd{}, []string{"s1", "Sheet1!B2:C3"}, ctx, flags); err != nil {
213+
t.Fatalf("notes: %v", err)
214+
}
215+
})
216+
217+
var result map[string]any
218+
if err := json.Unmarshal([]byte(out), &result); err != nil {
219+
t.Fatalf("unmarshal: %v (output: %q)", err, out)
220+
}
221+
222+
notes := result["notes"].([]any)
223+
first := notes[0].(map[string]any)
224+
if first["a1"] != "Sheet1!B2" {
225+
t.Errorf("expected a1 'Sheet1!B2', got %q", first["a1"])
226+
}
227+
if first["row"] != float64(2) {
228+
t.Errorf("expected row 2, got %v", first["row"])
229+
}
230+
if first["col"] != float64(2) {
231+
t.Errorf("expected col 2, got %v", first["col"])
232+
}
233+
}
234+
158235
func TestSheetsNotesCmd_NoNotes(t *testing.T) {
159236
origNew := newSheetsService
160237
t.Cleanup(func() { newSheetsService = origNew })
@@ -192,19 +269,19 @@ func TestSheetsNotesCmd_NoNotes(t *testing.T) {
192269
newSheetsService = func(context.Context, string) (*sheets.Service, error) { return svc, nil }
193270

194271
flags := &RootFlags{Account: "a@b.com"}
195-
u, uiErr := ui.New(ui.Options{Stdout: io.Discard, Stderr: io.Discard, Color: "never"})
196-
if uiErr != nil {
197-
t.Fatalf("ui.New: %v", uiErr)
198-
}
199-
ctx := ui.WithUI(context.Background(), u)
272+
errOut := captureStderr(t, func() {
273+
u, uiErr := ui.New(ui.Options{Stdout: io.Discard, Stderr: os.Stderr, Color: "never"})
274+
if uiErr != nil {
275+
t.Fatalf("ui.New: %v", uiErr)
276+
}
277+
ctx := ui.WithUI(context.Background(), u)
200278

201-
out := captureStdout(t, func() {
202279
if err := runKong(t, &SheetsNotesCmd{}, []string{"s1", "Sheet1!A1"}, ctx, flags); err != nil {
203280
t.Fatalf("notes: %v", err)
204281
}
205282
})
206283

207-
if strings.Contains(out, "ROW") {
208-
t.Errorf("expected no table output for empty notes: %q", out)
284+
if !strings.Contains(errOut, "No notes found") {
285+
t.Errorf("expected 'No notes found' on stderr: %q", errOut)
209286
}
210287
}

0 commit comments

Comments
 (0)