Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 44 additions & 9 deletions internal/cli/interactive.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -201,7 +201,7 @@ func runInteractive(p *ui.Printer, pr prompter, a *runDataIngestArgs, taskSet bo
p.PromptStep(2, 4, "Please name the dataset.")
p.Newline()
ans, err := pr.Input("Please name the dataset.",
"letters, digits, and underscores; start with a letter or underscore e.g. churn_train", "",
"letters, digits, and underscores — no hyphens or spaces, use _; start with a letter or underscore e.g. churn_train", "",
push.ValidateTableName)
if err != nil {
return err
Expand All@@ -224,12 +224,16 @@ func runInteractive(p *ui.Printer, pr prompter, a *runDataIngestArgs, taskSet bo
if err != nil {
return err
}
// Trim before storing: validateDatasetPath only trims to check for
// emptiness, so a pasted " ~/data" (stray leading/trailing space)
// would otherwise survive here and defeat expandHome (first char
// isn't '~') — filepath.Abs then prepends cwd and the sniff / label
// preview read a path that doesn't exist.
a.LocalPath = strings.TrimSpace(ans)
// Canonicalize before storing: dequotePath trims stray leading/
// trailing space (a pasted " ~/data" would otherwise defeat expandHome,
// whose first char isn't '~', and filepath.Abs would prepend cwd so the
// sniff / label preview read a path that doesn't exist) AND strips one
// matching pair of surrounding quotes. Dragging a folder into a terminal
// auto-quotes it, and users habitually quote paths with spaces; this
// prompt is read literally, not shell-parsed, so the quotes would
// otherwise become part of the path (#386). validateDatasetPath applies
// the same canonicalization, so the re-prompt guard stays consistent.
a.LocalPath = dequotePath(ans)
prompted = true
}
// Expand a leading ~ now so the family sniff + label-header preview read
Expand DownExpand Up@@ -576,14 +580,45 @@ func renderReview(p *ui.Printer, a *runDataIngestArgs) {
// validateDatasetPath rejects an empty / whitespace-only answer. Without
// it, a bare Enter at the path prompt yields "" — and SniffFamily(Abs(""))
// would sniff the current working directory before any empty-path guard
// runs, silently ingesting whatever happens to sit in the cwd.
// runs, silently ingesting whatever happens to sit in the cwd. It validates
// the canonicalized value (dequotePath) so an answer that is nothing but an
// empty pair of quotes is rejected at the prompt rather than surviving to
// statDatasetPath with a messy error (#386).
func validateDatasetPath(s string) error {
if strings.TrimSpace(s) == "" {
if dequotePath(s) == "" {
return fmt.Errorf("a dataset path is required")
}
return nil
}

// dequotePath canonicalizes an interactive path answer: it trims surrounding
// whitespace, then strips one matching pair of surrounding quotes (via
// stripSurroundingQuotes), then trims again in case the quotes wrapped padding.
// It is applied ONLY to the guided prompt answer, never to the flag/positional
// path — the shell already de-quotes those.
func dequotePath(s string) string {
return strings.TrimSpace(stripSurroundingQuotes(strings.TrimSpace(s)))
}

// stripSurroundingQuotes removes at most one matching pair of surrounding
// single or double quotes from s, and only when the first and last rune are
// the SAME quote char (' or ') and s has at least two runes. Inner quotes are
// left untouched, mismatched quotes ('…" ) are left as-is, and a path whose
// real name contains a quote loses only that single matched outer pair. This
// exists because the interactive path prompt is read literally, not
// shell-parsed, so a pasted quoted path would otherwise fail to resolve (#386).
func stripSurroundingQuotes(s string) string {
r := []rune(s)
if len(r) < 2 {
return s
}
first, last := r[0], r[len(r)-1]
if (first == '\'' || first == '"') && first == last {
return string(r[1 : len(r)-1])
}
return s
}

// validatePositiveInt accepts a string that parses to an int > 0.
func validatePositiveInt(s string) error {
if n, err := strconv.Atoi(strings.TrimSpace(s)); err != nil || n <= 0 {
Expand Down
68 changes: 68 additions & 0 deletions internal/cli/interactive_test.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -644,3 +644,71 @@ func orderedSubsequence(got, want []string) bool {
}
return i == len(want)
}

// TestStripSurroundingQuotes / TestDequotePath / TestValidateDatasetPathDequote
// pin the interactive path-prompt quote handling (#386): a pasted quoted path
// must resolve identically to the bare path, an unquoted path with spaces must
// survive untouched, and a name that really contains a quote must not be
// corrupted beyond the single matched outer pair.
func TestStripSurroundingQuotes(t *testing.T) {
cases := []struct {
name string
in string
want string
}{
{"single-quoted", "'/a/b'", "/a/b"},
{"double-quoted", `"/a/b"`, "/a/b"},
{"bare", "/a/b", "/a/b"},
{"bare-with-spaces", "/a/b c/train", "/a/b c/train"},
{"quoted-with-spaces", "'/home/me/my data/train'", "/home/me/my data/train"},
{"name-contains-a-quote-unquoted", "/a/b's", "/a/b's"},
{"name-contains-a-quote-quoted-strips-one-pair", "'/a/b's'", "/a/b's"},
{"inner-quotes-left-untouched", `/a/"b"/c`, `/a/"b"/c`},
{"mismatched-quotes-left-untouched", `'/a/b"`, `'/a/b"`},
{"single-char-not-stripped", "'", "'"},
{"empty", "", ""},
{"empty-single-quotes", "''", ""},
{"empty-double-quotes", `""`, ""},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
if got := stripSurroundingQuotes(tc.in); got != tc.want {
t.Errorf("stripSurroundingQuotes(%q) = %q, want %q", tc.in, got, tc.want)
}
})
}
}

func TestDequotePath(t *testing.T) {
cases := []struct {
name string
in string
want string
}{
{"quoted-with-surrounding-space", " '/a/b' ", "/a/b"},
{"double-quoted", `"/a/b"`, "/a/b"},
{"bare-with-interior-spaces", " /a/b c ", "/a/b c"},
{"quoted-path-with-spaces", "'/home/me/my data/train'", "/home/me/my data/train"},
{"only-quotes", "''", ""},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
if got := dequotePath(tc.in); got != tc.want {
t.Errorf("dequotePath(%q) = %q, want %q", tc.in, got, tc.want)
}
})
}
}

func TestValidateDatasetPathRejectsEmptyQuotes(t *testing.T) {
for _, in := range []string{"", " ", "''", `""`, " '' "} {
if err := validateDatasetPath(in); err == nil {
t.Errorf("validateDatasetPath(%q) = nil, want a required-path error", in)
}
}
for _, in := range []string{"'/a/b'", "/a/b", "/a/b c"} {
if err := validateDatasetPath(in); err != nil {
t.Errorf("validateDatasetPath(%q) = %v, want nil", in, err)
}
}
}
2 changes: 1 addition & 1 deletion internal/cli/testdata/golden/zz-all-strings.golden
Original file line numberDiff line numberDiff line change
Expand Up@@ -460,7 +460,7 @@ screen. %s/%d are runtime placeholders.
"label column"
"label policy"
"labels.csv"
"letters, digits, and underscores; start with a letter or underscore e.g. churn_train"
"letters, digits, and underscores — no hyphens or spaces, use _; start with a letter or underscore e.g. churn_train"
"listing Pods for service %s/%s: %w"
"listing chart-managed deployments in namespace %s: %w"
"listing client deployments to check for an existing client: %w"
Expand Down
3 changes: 2 additions & 1 deletion internal/push/spec.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -111,7 +111,8 @@ func ValidateTableName(table string) error {
}
if !tableNamePattern.MatchString(table) {
return fmt.Errorf(
"%q won't work — use letters, digits, and underscores, "+
"%q won't work — no hyphens or spaces (use _); "+
"use letters, digits, and underscores, "+
"starting with a letter or underscore (e.g. churn_train)",
table)
}
Expand Down
17 changes: 17 additions & 0 deletions internal/push/spec_test.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -666,3 +666,20 @@ func TestBuild_EmitsDetectedExtension_PassesSchema(t *testing.T) {
})
}
}

// TestValidateTableName_ErrorMentionsHyphensAndSpaces pins the #386 wording:
// the pattern-rejection error's first clause must spell out that hyphens and
// spaces are not allowed and that _ is the separator — kebab-case (tsc-train)
// is the common mistake, so the rejection has to say so up front.
func TestValidateTableName_ErrorMentionsHyphensAndSpaces(t *testing.T) {
err := ValidateTableName("tsc-train")
if err == nil {
t.Fatal("ValidateTableName(\"tsc-train\") = nil, want a rejection")
}
msg := err.Error()
for _, want := range []string{"no hyphens or spaces", "use _"} {
if !strings.Contains(msg, want) {
t.Errorf("error %q does not mention %q", msg, want)
}
}
}
Loading