Make the state publish meta data clearer to review and edit

changelog.md

@@ -26,16 +26,26 @@ and this project adheres to
   it to a meta file to edit and publish later with `--meta`, instead of only cancelling.
 - When `state publish --build` cannot determine the ingredient name or version, the error now says which is missing,
   why `pyproject.toml` did not provide it (for example, a version listed in `[project].dynamic`), and how to set it.
+- Without `--namespace`, `state publish --build` now defaults to `private/<org>/python` when the source has a
+  `pyproject.toml` (otherwise `private/<org>`, as before).
+- The namespace of a new ingredient can now be changed in the `state publish` editor. The new namespace is checked like
+  one passed with `--namespace`; if it can't be used, the previous one is kept and the reason is shown. The confirmation
+  and the editor now say up front which fields can't be changed and why, and when one of them is edited anyway (the
+  name or version with `--build`, or the namespace, name or version with `--edit`), the error names it.
+- The ingredient meta data `state publish` shows for confirmation and opens in the editor no longer has a help comment
+  on each field. The fields are described in the publish guide instead.
 
 ### Fixed
 
 - `state publish --edit` no longer overwrites the meta file's dependencies and authors with the previous revision's,
-  keeps inherited dependency types and conditions, and rejects author or description changes on an existing version
-  up front.
+  keeps inherited dependency types and conditions, and rejects author changes on an existing version up front.
 - `state publish --edit --meta` with an exported meta file now edits the version named in the file rather than the
   latest version.
 - `state export ingredient-meta` now writes plain YAML without help comments or line wrapping, so its output can be
   passed back with `--meta` as is.
+- `state export ingredient-meta` now includes the ingredient's description instead of leaving it empty. When
+  `state publish --edit` is given a different description for an existing version, it no longer refuses to publish:
+  the confirmation says the description can't be changed, shows the current one, and the change is ignored.
 - `state publish` of a new ingredient without a namespace (such as with `--editor` outside a project) no longer searches
   every namespace for its name, which could time out with "Could not search for ingredient" before the editor opened.
 - `state publish --editor` no longer fails with "Could not publish ingredient. Error received: <no value>" when no editor

go.mod

@@ -76,7 +76,6 @@ require (
 	github.com/gowebpki/jcs v1.0.1
 	github.com/mark3labs/mcp-go v0.33.0
 	github.com/mholt/archives v0.1.5
-	github.com/zijiren233/yaml-comment v0.2.1
 )
 
 require (
@@ -118,7 +117,6 @@ require (
 	github.com/klauspost/pgzip v1.2.6 // indirect
 	github.com/kr/pty v1.1.8 // indirect
 	github.com/lucasb-eyer/go-colorful v1.2.0 // indirect
-	github.com/maruel/natural v1.1.0 // indirect
 	github.com/mattn/go-localereader v0.0.1 // indirect
 	github.com/mikelolasagasti/xz v1.0.1 // indirect
 	github.com/minio/minlz v1.0.1 // indirect

go.sum

@@ -520,8 +520,6 @@ github.com/mark3labs/mcp-go v0.33.0 h1:naxhjnTIs/tyPZmWUZFuG0lDmdA6sUyYGGf3gsHvT
 github.com/mark3labs/mcp-go v0.33.0/go.mod h1:rXqOudj/djTORU/ThxYx8fqEVj/5pvTuuebQ2RC7uk4=
 github.com/markbates/oncer v0.0.0-20181203154359-bf2de49a0be2/go.mod h1:Ld9puTsIW75CHf65OeIOkyKbteujpZVXDpWK6YGZbxE=
 github.com/markbates/safe v1.0.1/go.mod h1:nAqgmRi7cY2nqMc92/bSEeQA+R4OheNU2T1kNSCBdG0=
-github.com/maruel/natural v1.1.0 h1:2z1NgP/Vae+gYrtC0VuvrTJ6U35OuyUqDdfluLqMWuQ=
-github.com/maruel/natural v1.1.0/go.mod h1:eFVhYCcUOfZFxXoDZam8Ktya72wa79fNC3lc/leA0DQ=
 github.com/mash/go-tempfile-suffix v0.0.0-20150731093933-48f0f8a3a5ab h1:2lWb5W+cZTlTIrJUuzVMrs6vcEvY0ydkkMH66EdPaqc=
 github.com/mash/go-tempfile-suffix v0.0.0-20150731093933-48f0f8a3a5ab/go.mod h1:oBJAJTenVXse2frk3J4anZ7funxla2ZdJHFQShkD43k=
 github.com/matryer/is v1.4.1 h1:55ehd8zaGABKLXQUe2awZ99BD/PTc2ls+KV/dXphgEQ=
@@ -745,8 +743,6 @@ github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9dec
 github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
 github.com/yusufpapurcu/wmi v1.2.4 h1:zFUKzehAFReQwLys1b/iSMl+JQGSCSjtVqQn9bBrPo0=
 github.com/yusufpapurcu/wmi v1.2.4/go.mod h1:SBZ9tNy3G9/m5Oi98Zks0QjeHVDvuK0qfxQmPyzfmi0=
-github.com/zijiren233/yaml-comment v0.2.1 h1:/ymMfauuR6zPme+c59FvGNmvxmjOS+BRZSU9YEM82g4=
-github.com/zijiren233/yaml-comment v0.2.1/go.mod h1:Cps/tIAJVNoG6TpNN1Aaht/O1QGthZzOgUMQXXTxlBs=
 go.etcd.io/bbolt v1.3.2/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU=
 go.mongodb.org/mongo-driver v1.0.3/go.mod h1:u7ryQJ+DOzQmeO7zB6MHyr8jkEQvC8vH7qLUO4lqsUM=
 go.mongodb.org/mongo-driver v1.1.1/go.mod h1:u7ryQJ+DOzQmeO7zB6MHyr8jkEQvC8vH7qLUO4lqsUM=

internal/runners/export/ingredientmeta.go

@@ -8,7 +8,6 @@ import (
 	"github.com/ActiveState/cli/internal/locale"
 	"github.com/ActiveState/cli/internal/runners/publish"
 	"github.com/ActiveState/cli/pkg/platform/api/graphql/request"
-	"gopkg.in/yaml.v3"
 )
 
 type IngredientMetaParams struct {
@@ -37,11 +36,10 @@ func (i *IngredientMeta) Run(params *IngredientMetaParams) error {
 	return writeIngredientMeta(i.prime.Output().Config().OutWriter, meta)
 }
 
-// writeIngredientMeta writes meta in the format --meta reads. It is plain yaml rather than meta.MarshalYaml, whose field
-// help comments would end up in the file, and written as is: the outputer's Print would word-wrap long lines and
-// interpret [TAGS].
+// writeIngredientMeta writes meta in the format --meta reads. It is written as is: the outputer's Print would word-wrap
+// long lines and interpret [TAGS].
 func writeIngredientMeta(w io.Writer, meta *request.PublishVariables) error {
-	b, err := yaml.Marshal(meta)
+	b, err := meta.MarshalYaml(false)
 	if err != nil {
 		return errs.Wrap(err, "Could not marshal ingredient meta data")
 	}

internal/runners/publish/confirm_test.go

@@ -93,7 +93,7 @@ func TestConfirmRequestFillsVersionRequirements(t *testing.T) {
 	r := &Runner{out: outputhelper.NewCatcher(), prompt: p}
 	reqVars := newRequest()
 
-	publish, err := r.confirmRequest(reqVars, &Params{}, false)
+	publish, err := r.confirmRequest(reqVars, &Params{}, false, "")
 	if err != nil || !publish {
 		t.Fatalf("publish = %v, err = %v", publish, err)
 	}
@@ -104,6 +104,9 @@ func TestConfirmRequestFillsVersionRequirements(t *testing.T) {
 	if !strings.Contains(p.messages[0], "versionRequirements: '>=0'") {
 		t.Errorf("confirmation does not show the filled in requirements:\n%s", p.messages[0])
 	}
+	if strings.Contains(p.messages[0], "#") {
+		t.Errorf("confirmation shows comments:\n%s", p.messages[0])
+	}
 }
 
 func TestOpenInEditorWithoutEditor(t *testing.T) {
@@ -130,7 +133,7 @@ func TestConfirmRequestEditor(t *testing.T) {
 		r := &Runner{out: outputhelper.NewCatcher(), prompt: p}
 		reqVars := newRequest()
 
-		publish, err := r.confirmRequest(reqVars, &Params{}, false)
+		publish, err := r.confirmRequest(reqVars, &Params{}, false, "")
 		if err != nil || !publish {
 			t.Fatalf("publish = %v, err = %v", publish, err)
 		}
@@ -139,25 +142,110 @@ func TestConfirmRequestEditor(t *testing.T) {
 		}
 	})
 
-	t.Run("cleared namespace is rejected", func(t *testing.T) {
-		useTestEditor(t, "name: mypkg\nversion: 1.0.0\n")
+	t.Run("namespace change is kept", func(t *testing.T) {
+		useTestEditor(t, "name: mypkg\nnamespace: private/org/other\nversion: 1.0.0\n")
+		p := &fakePrompter{confirms: []bool{false, true}, selects: []int{choiceEdit}}
+		r := &Runner{out: outputhelper.NewCatcher(), prompt: p, versionExists: noVersions}
+		reqVars := newRequest()
+
+		publish, err := r.confirmRequest(reqVars, &Params{}, false, "")
+		if err != nil || !publish {
+			t.Fatalf("publish = %v, err = %v", publish, err)
+		}
+		if reqVars.Namespace != "private/org/other" {
+			t.Errorf("namespace = %q, want the edited one", reqVars.Namespace)
+		}
+	})
+
+	for name, tt := range map[string]struct {
+		namespace string
+		params    Params
+		exists    bool
+		reason    string
+	}{
+		"cleared namespace is put back":          {"", Params{}, false, "namespace is required"},
+		"namespace with the version is put back": {"private/org/other", Params{}, true, "already exists"},
+		"namespace outside the org is put back":  {"private/other/python", Params{Build: "src"}, false, "requires a namespace under"},
+	} {
+		t.Run(name, func(t *testing.T) {
+			useTestEditor(t, "name: mypkg\nnamespace: "+tt.namespace+"\nversion: 1.0.0\n")
+			out := outputhelper.NewCatcher()
+			p := &fakePrompter{confirms: []bool{false, true}, selects: []int{choiceEdit}}
+			r := &Runner{out: out, prompt: p, versionExists: func(_, _, _ string) (bool, error) { return tt.exists, nil }}
+			reqVars := newRequest()
+
+			publish, err := r.confirmRequest(reqVars, &tt.params, false, "")
+			if err != nil || !publish {
+				t.Fatalf("publish = %v, err = %v", publish, err)
+			}
+			if reqVars.Namespace != "private/org/python" {
+				t.Errorf("namespace = %q, want the original one put back", reqVars.Namespace)
+			}
+			if !strings.Contains(out.CombinedOutput(), tt.reason) {
+				t.Errorf("output does not explain why (%q):\n%s", tt.reason, out.CombinedOutput())
+			}
+		})
+	}
+
+	t.Run("name change with --build is saved instead", func(t *testing.T) {
+		t.Chdir(t.TempDir())
+		useTestEditor(t, "name: otherpkg\nnamespace: private/org/python\nversion: 1.0.0\n")
 		p := &fakePrompter{confirms: []bool{false}, selects: []int{choiceEdit}}
 		r := &Runner{out: outputhelper.NewCatcher(), prompt: p}
 
-		_, err := r.confirmRequest(newRequest(), &Params{}, false)
-		if err == nil || !strings.Contains(err.Error(), "namespace") {
-			t.Errorf("err = %v, want the namespace error", err)
+		_, err := r.confirmRequest(newRequest(), &Params{Build: "src"}, false, "")
+		if err == nil || !strings.Contains(err.Error(), "The name can't be changed") {
+			t.Fatalf("err = %v, want the name named as fixed", err)
+		}
+		if !strings.Contains(p.messages[0], "name and version are built into the package") {
+			t.Errorf("confirmation does not say up front what is fixed:\n%s", p.messages[0])
+		}
+		if _, err := os.Stat("otherpkg-1.0.0.meta.yaml"); err != nil {
+			t.Errorf("edits were not saved: %v", err)
 		}
 	})
 
-	t.Run("description added on a revision is rejected", func(t *testing.T) {
-		useTestEditor(t, "name: mypkg\nnamespace: private/org/python\nversion: 1.0.0\ndescription: changed\n")
+	t.Run("namespace change with --edit is saved instead", func(t *testing.T) {
+		t.Chdir(t.TempDir())
+		useTestEditor(t, "name: mypkg\nnamespace: private/org/other\nversion: 1.0.0\n")
 		p := &fakePrompter{confirms: []bool{false}, selects: []int{choiceEdit}}
 		r := &Runner{out: outputhelper.NewCatcher(), prompt: p}
 
-		_, err := r.confirmRequest(newRequest(), &Params{Edit: true}, true)
-		if err == nil || !strings.Contains(err.Error(), "cannot be changed") {
-			t.Errorf("err = %v, want the identity error", err)
+		_, err := r.confirmRequest(newRequest(), &Params{Edit: true}, true, "")
+		if err == nil || !strings.Contains(err.Error(), "The namespace can't be changed") {
+			t.Errorf("err = %v, want the namespace named as fixed", err)
+		}
+	})
+
+	t.Run("changed description on a revision is ignored", func(t *testing.T) {
+		useTestEditor(t, "name: mypkg\nnamespace: private/org/python\nversion: 1.0.0\ndescription: changed\n")
+		p := &fakePrompter{confirms: []bool{false, true}, selects: []int{choiceEdit}}
+		r := &Runner{out: outputhelper.NewCatcher(), prompt: p}
+		reqVars := newRequest()
+
+		publish, err := r.confirmRequest(reqVars, &Params{Edit: true}, true, "current")
+		if err != nil || !publish {
+			t.Fatalf("publish = %v, err = %v, want it still published", publish, err)
+		}
+		if reqVars.Description != "current" {
+			t.Errorf("description = %q, want the current one kept", reqVars.Description)
+		}
+		if !strings.Contains(p.messages[1], "changed[/RESET]' is ignored and it stays '[ACTIONABLE]current") {
+			t.Errorf("confirmation does not say the change is ignored:\n%s", p.messages[1])
+		}
+	})
+
+	t.Run("unchanged description on a revision is accepted", func(t *testing.T) {
+		useTestEditor(t, "name: mypkg\nnamespace: private/org/python\nversion: 1.0.0\ndescription: current\n")
+		p := &fakePrompter{confirms: []bool{false, true}, selects: []int{choiceEdit}}
+		r := &Runner{out: outputhelper.NewCatcher(), prompt: p}
+
+		publish, err := r.confirmRequest(newRequest(), &Params{Edit: true}, true, "current")
+		if err != nil || !publish {
+			t.Errorf("publish = %v, err = %v, want the unchanged description accepted", publish, err)
+		}
+		if strings.Contains(p.messages[1], "is ignored") {
+			t.Errorf("confirmation notes an ignored description that wasn't changed:\n%s", p.messages[1])
 		}
 	})
 
@@ -167,7 +255,7 @@ func TestConfirmRequestEditor(t *testing.T) {
 		r := &Runner{out: outputhelper.NewCatcher(), prompt: p}
 		reqVars := newRequest()
 
-		if _, err := r.confirmRequest(reqVars, &Params{Build: "src"}, false); err != nil {
+		if _, err := r.confirmRequest(reqVars, &Params{Build: "src"}, false, ""); err != nil {
 			t.Fatal(err)
 		}
 		if len(reqVars.Dependencies) != 1 || reqVars.Dependencies[0].Name != privateBuilderName {
@@ -176,12 +264,31 @@ func TestConfirmRequestEditor(t *testing.T) {
 	})
 }
 
+// noVersions is a versionExists for a platform where nothing is published yet.
+func noVersions(_, _, _ string) (bool, error) { return false, nil }
+
+func TestDefaultNamespace(t *testing.T) {
+	withPyproject := t.TempDir()
+	if err := os.WriteFile(filepath.Join(withPyproject, "pyproject.toml"), nil, 0644); err != nil {
+		t.Fatal(err)
+	}
+	for _, tt := range []struct{ buildDir, want string }{
+		{"", "private/org"},
+		{t.TempDir(), "private/org"},
+		{withPyproject, "private/org/python"},
+	} {
+		if got := defaultNamespace("org", tt.buildDir); got != tt.want {
+			t.Errorf("defaultNamespace(%q) = %q, want %q", tt.buildDir, got, tt.want)
+		}
+	}
+}
+
 func TestConfirmRequestSave(t *testing.T) {
 	t.Chdir(t.TempDir())
 	p := &fakePrompter{confirms: []bool{false}, selects: []int{choiceSave}}
 	r := &Runner{out: outputhelper.NewCatcher(), prompt: p}
 
-	publish, err := r.confirmRequest(newRequest(), &Params{Build: "src"}, false)
+	publish, err := r.confirmRequest(newRequest(), &Params{Build: "src"}, false, "")
 	if err != nil || publish {
 		t.Fatalf("publish = %v, err = %v, want saved", publish, err)
 	}
@@ -207,7 +314,7 @@ func TestConfirmRequestCancel(t *testing.T) {
 	p := &fakePrompter{confirms: []bool{false}, selects: []int{choiceCancel}}
 	r := &Runner{out: outputhelper.NewCatcher(), prompt: p}
 
-	if _, err := r.confirmRequest(newRequest(), &Params{}, false); err == nil || !strings.Contains(err.Error(), "cancelled") {
+	if _, err := r.confirmRequest(newRequest(), &Params{}, false, ""); err == nil || !strings.Contains(err.Error(), "cancelled") {
 		t.Errorf("err = %v, want cancelled", err)
 	}
 }
@@ -222,7 +329,7 @@ func TestPublishFailedError(t *testing.T) {
 func TestMetaFromIngredient(t *testing.T) {
 	ingredient := &ParentIngredient{Version: "1.0.0", Dependencies: windowsOnlyDependency()}
 
-	meta, err := metaFromIngredient("private/org/python", "mypkg", ingredient)
+	meta, err := metaFromIngredient("private/org/python", "mypkg", ingredient, "Internal helpers")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -232,6 +339,9 @@ func TestMetaFromIngredient(t *testing.T) {
 	if got := meta.Dependencies[0].Conditions[0].VersionRequirements; got != ">=0" {
 		t.Errorf("condition without requirements exported as %q, want >=0", got)
 	}
+	if meta.Description != "Internal helpers" {
+		t.Errorf("description exported as %q", meta.Description)
+	}
 }
 
 // windowsOnlyDependency is a revision's dependency on requests, conditioned on Windows without a version requirement.

internal/runners/publish/meta.go

@@ -5,16 +5,18 @@ import (
 
 	"github.com/ActiveState/cli/internal/errs"
 	"github.com/ActiveState/cli/internal/locale"
+	"github.com/ActiveState/cli/internal/rtutils/ptr"
 	"github.com/ActiveState/cli/pkg/platform/api/graphql/request"
 	"github.com/ActiveState/cli/pkg/platform/api/inventory/inventory_client/inventory_operations"
 	"github.com/ActiveState/cli/pkg/platform/authentication"
 	"github.com/ActiveState/cli/pkg/platform/model"
+	"github.com/go-openapi/strfmt"
 )
 
 // FetchMeta returns the meta file contents of an existing ingredient version (the latest one when version is empty),
 // in the format the --meta flag accepts. Its dependencies are the complete set, so the file can be modified and
-// passed back with --edit to replace them. Authors and description are left out because the platform rejects them
-// on a revision of an existing version.
+// passed back with --edit to replace them. Authors are left out because the platform rejects them on a revision of an
+// existing version; the description is included, and --edit accepts it as long as it is unchanged.
 func FetchMeta(namespace, name, version string, auth *authentication.Auth) (*request.PublishVariables, error) {
 	latestRevisionTime, err := model.FetchLatestRevisionTimeStamp(auth)
 	if err != nil {
@@ -48,13 +50,26 @@ func FetchMeta(namespace, name, version string, auth *authentication.Auth) (*req
 		ingredient = &ParentIngredient{*i.IngredientID, *i.IngredientVersionID, *i.Version, i.Dependencies}
 	}
 
-	return metaFromIngredient(namespace, name, ingredient)
+	description, err := fetchDescription(ingredient.IngredientID, auth)
+	if err != nil {
+		return nil, err
+	}
+	return metaFromIngredient(namespace, name, ingredient, description)
+}
+
+// fetchDescription returns an ingredient's description, which belongs to the ingredient rather than to its versions.
+func fetchDescription(ingredientID strfmt.UUID, auth *authentication.Auth) (string, error) {
+	ingredient, err := model.FetchIngredient(&ingredientID, auth)
+	if err != nil {
+		return "", locale.WrapError(err, "err_ingredient_meta_fetch", "Could not fetch the ingredient's description")
+	}
+	return ptr.From(ingredient.Description, ""), nil
 }
 
 // metaFromIngredient builds the meta file contents of an existing ingredient version, complete enough to be published
 // back as is: dependencies and conditions without a version requirement get the any-version one.
-func metaFromIngredient(namespace, name string, ingredient *ParentIngredient) (*request.PublishVariables, error) {
-	meta := &request.PublishVariables{Name: name, Namespace: namespace}
+func metaFromIngredient(namespace, name string, ingredient *ParentIngredient, description string) (*request.PublishVariables, error) {
+	meta := &request.PublishVariables{Name: name, Namespace: namespace, Description: description}
 	// As a revision, prepareEditRequest doesn't fetch authors, so it needs no auth.
 	if err := prepareEditRequest(ingredient, meta, true, nil); err != nil {
 		return nil, errs.Wrap(err, "Could not read ingredient meta data")