From 83baf2f0aabe22af86ce269e0daa787dea5333e1 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 24 Sep 2026 20:54:17 +0000 Subject: [PATCH 1/7] fix(doc): let a table column hold as many values as its feature declares A computed column reading a feature or parameter now keeps every value the row binds, bounded by the declared multiplicity, so a [0..*] attribute renders as its ordered values and an unbound optional one as an empty cell; a scalar declaration still fails the column, naming the declared bound. Co-Authored-By: jason.han --- docs/manual/query-cookbook.md | 12 +- internal/doc/docrender/html_test.go | 16 +++ internal/doc/docrender/markdown_test.go | 12 ++ .../testdata/collection_report.sysml | 40 +++++++ internal/doc/queryexec/computed.go | 59 +++++++-- internal/doc/queryexec/computed_test.go | 112 ++++++++++++++++-- internal/doc/queryexec/documentation_test.go | 25 ++-- internal/doc/queryexec/errors.go | 3 +- internal/ir/queryplan/columns.go | 16 +-- internal/ir/queryplan/compiler.go | 28 ++--- internal/ir/queryplan/plan.go | 48 ++++++-- internal/semantic/semantics/collection.go | 9 ++ 12 files changed, 309 insertions(+), 71 deletions(-) create mode 100644 internal/doc/docrender/testdata/collection_report.sysml diff --git a/docs/manual/query-cookbook.md b/docs/manual/query-cookbook.md index 8f645d75fd..f04014a2a2 100644 --- a/docs/manual/query-cookbook.md +++ b/docs/manual/query-cookbook.md @@ -739,10 +739,14 @@ for the two connections in its results. Computed names join the projection: Every built-in property is reachable the same way — `Element::shortName`, `Element::declaredShortName` and `Element::documentation` included — so `(Element::shortName ?? "—") + ": " + Element::name` labels a row by its -identifier. A column is one value per row: an element carrying two `doc` -bodies fails a column over `Element::documentation` with a typed -`column-cardinality` error, where the plain `"documentation"` projection -above carries both. +identifier. A column holds as many values as the feature it reads declares: +`Element::documentation` is `[0..*]`, so an element carrying two `doc` bodies +fills the cell with both in order (comma-joined in a document table) and one +carrying none leaves it empty, while a feature declared without a multiplicity +is one value per row — a row binding two fails the column with a typed +`column-cardinality` error naming the declared bound, and a row binding none +with `column-absent` unless `??` supplies a default. Operators always take one +value per operand, so `Element::documentation + "."` over two bodies fails. Quantities take part in column arithmetic with the runtime's rules, so a column keeps its unit: `Stage::mass * 2` is `4580000 [kg]`, `Stage::mass / diff --git a/internal/doc/docrender/html_test.go b/internal/doc/docrender/html_test.go index f1c86d9fa3..6311b48b53 100644 --- a/internal/doc/docrender/html_test.go +++ b/internal/doc/docrender/html_test.go @@ -746,3 +746,19 @@ func TestHTMLAnonymousSectionLeavesReservedAnchor(t *testing.T) { t.Errorf("the reference does not resolve to the named section:\n%s", got) } } + +// TestHTMLCollectionCells checks a `[0..*]` column: a row holding two values +// renders each as its own value in order, separated, and a row holding none +// renders an empty cell. +func TestHTMLCollectionCells(t *testing.T) { + got := renderFixtureHTML(t, filepath.Join("testdata", "collection_report.sysml"), + "Calibration::TimingReport", HTMLOptions{}) + for _, want := range []string{ + `69, 98`, + ``, + } { + if !strings.Contains(got, want) { + t.Errorf("rendering does not contain %q\n%s", want, got) + } + } +} diff --git a/internal/doc/docrender/markdown_test.go b/internal/doc/docrender/markdown_test.go index 5f3795a2fb..a44aafaa4c 100644 --- a/internal/doc/docrender/markdown_test.go +++ b/internal/doc/docrender/markdown_test.go @@ -285,6 +285,18 @@ func TestMarkdownQuantityReportGolden(t *testing.T) { } } +// TestMarkdownCollectionCells checks a `[0..*]` column: a row holding two values +// renders them comma-joined in order and a row holding none renders empty. +func TestMarkdownCollectionCells(t *testing.T) { + got := renderFixtureDocument(t, + filepath.Join("testdata", "collection_report.sysml"), + "Calibration::TimingReport") + want := "| name | durations | label |\n| --- | --- | --- |\n| nominal | 69, 98 | nominal |\n| idle | | idle |\n" + if !strings.Contains(got, want) { + t.Errorf("rendering does not contain %q\n%s", want, got) + } +} + // TestMarkdownDerivedReportGolden locks a document whose table, list and // definitions read attributes derived from other features: sums of sibling // masses through type- and usage-level redefinitions, chains into owned parts, diff --git a/internal/doc/docrender/testdata/collection_report.sysml b/internal/doc/docrender/testdata/collection_report.sysml new file mode 100644 index 0000000000..a7c662c2ea --- /dev/null +++ b/internal/doc/docrender/testdata/collection_report.sysml @@ -0,0 +1,40 @@ +package Calibration { + private import DocumentQueries::*; + private import KerML::Root::Element; + private import ScalarValues::*; + + part def Scenario { + attribute durations : Real[0..*]; + attribute label : String; + } + part campaign { + part nominal : Scenario { + attribute :>> durations = (69.0, 98.0); + attribute :>> label = "nominal"; + } + part idle : Scenario { + attribute :>> label = "idle"; + } + } + + calc def Timings :> Query { + in root : Element = campaign; + Project( + source = WhereType(source = Descendants(source = root, maxDepth = 1), type = "PartUsage"), + properties = ("name"), + columns = ( + Column(name = "durations", expression = Scenario::durations), + Column(name = "label", expression = Scenario::label) + ) + ) + } + + part def TimingReport :> Document { + attribute redefines title = "Calibration Timings"; + + part timings : Table { + attribute redefines caption = "Durations per scenario"; + calc rows : Timings; + } + } +} diff --git a/internal/doc/queryexec/computed.go b/internal/doc/queryexec/computed.go index 492a7e8cba..0f7a7eabda 100644 --- a/internal/doc/queryexec/computed.go +++ b/internal/doc/queryexec/computed.go @@ -85,8 +85,9 @@ func (t *propertyTracker) missing() (string, bool) { return "", false } -// evaluateColumnCell evaluates one computed column for one row element. -// A failure or absent final result fails the query; ?? defaults absence. +// evaluateColumnCell evaluates one computed column for one row element: the +// cell holds the expression's values in order, as many as the feature it reads +// declares. A count outside that multiplicity fails the query; ?? defaults absence. func (e *executor) evaluateColumnCell( column computedColumn, row Value, @@ -99,16 +100,58 @@ func (e *executor) evaluateColumnCell( if err != nil { return nil, err } + multiplicity := columnMultiplicity(column.expression) + if multiplicity.Admits(len(values)) { + return values, nil + } if len(values) == 0 { return nil, e.columnError( ErrorColumnAbsent, column.name, row, column.expression.Origin(), "", "") } - if len(values) > 1 { - return nil, e.columnError( - ErrorColumnCardinality, column.name, row, column.expression.Origin(), - "", strconv.Itoa(len(values))) + failure := e.columnError( + ErrorColumnCardinality, column.name, row, column.expression.Origin(), + "", strconv.Itoa(len(values))) + failure.Expected = multiplicity.String() + return nil, failure +} + +// columnMultiplicity is how many values a column expression may produce: what +// the feature or parameter it reads declares, one for an operator's result, +// and a literal's own count; `a ?? b` admits either operand's count. +func columnMultiplicity(expression queryplan.Expression) queryplan.Multiplicity { + one := queryplan.Multiplicity{Lower: 1, Upper: 1, Known: true} + switch expression.Operation() { + case queryplan.OperationRowProperty, queryplan.OperationParameter: + return expression.Multiplicity() + case queryplan.OperationLiteral: + if kind, _ := expression.Literal(); kind == queryplan.LiteralNull { + return queryplan.Multiplicity{Known: true} + } + return one + case queryplan.OperationColumnOperator: + if _, operator := expression.Literal(); operator != "??" { + return one + } + operands := expression.Arguments() + return coalescedMultiplicity( + columnMultiplicity(operands[0].Value), columnMultiplicity(operands[1].Value)) + default: + return queryplan.Multiplicity{} + } +} + +// coalescedMultiplicity bounds `a ?? b`: a present left operand holds at least +// one value, and an absent one yields the right operand's count. +func coalescedMultiplicity(left, right queryplan.Multiplicity) queryplan.Multiplicity { + if !left.Known || !right.Known { + return queryplan.Multiplicity{} + } + return queryplan.Multiplicity{ + Lower: min(max(left.Lower, 1), right.Lower), + Upper: max(left.Upper, right.Upper), + UpperInfinite: left.UpperInfinite || right.UpperInfinite, + Known: true, } - return values, nil } func (e *executor) evaluateColumnExpression( @@ -582,7 +625,7 @@ func (e *executor) columnError( origin symbols.Origin, operator string, actual string, -) error { +) *Error { return &Error{ Kind: kind, Query: e.definition.Name(), diff --git a/internal/doc/queryexec/computed_test.go b/internal/doc/queryexec/computed_test.go index 90f15d5592..a625dc3b9e 100644 --- a/internal/doc/queryexec/computed_test.go +++ b/internal/doc/queryexec/computed_test.go @@ -3,6 +3,7 @@ package queryexec import ( "errors" "slices" + "strings" "testing" ) @@ -271,9 +272,10 @@ calc def Levels :> Query { } } -func TestExecuteComputedMultiValuedParameterIsTyped(t *testing.T) { +// A column reading a multi-valued parameter holds every bound value, in order. +func TestExecuteComputedMultiValuedParameterFillsCell(t *testing.T) { fixture := computedFixture(t, ` -calc def Bad :> Query { +calc def Factors :> Query { in root : Element; in factors : Real[0..*]; Project( @@ -281,20 +283,39 @@ calc def Bad :> Query { columns = (Column(name = "f", expression = factors)) ) }`) - _, err := fixture.execute(t, "Bad", Bindings{ + result, err := fixture.execute(t, "Factors", Bindings{ "root": {ElementValue(fixture.symbol(t, "system"))}, "factors": {RealValue(1.0), RealValue(2.0)}, }, Options{}) - var executionError *Error - if !errors.As(err, &executionError) || executionError.Kind != ErrorColumnCardinality { - t.Fatalf("error = %v", err) + if err != nil { + t.Fatalf("execute: %v", err) } - if executionError.Property != "f" || executionError.Actual != "2" { - t.Fatalf("error provenance = %+v", executionError) + if got := cellReals(t, result, "f"); !slices.EqualFunc(got, [][]float64{{1, 2}, {1, 2}}, slices.Equal) { + t.Fatalf("f cells = %v, want both factors in order per row", got) } - if !executionError.Origin.Located() { - t.Fatal("cardinality errors must retain source provenance") +} + +// cellReals reads every real value of the named column, one slice per row; a +// row's slice is nil where the cell is empty. +func cellReals(t *testing.T, result *RowSet, column string) [][]float64 { + t.Helper() + position := slices.IndexFunc(result.Columns(), func(c Column) bool { return c.Name() == column }) + if position < 0 { + t.Fatalf("no column %s in %v", column, result.Columns()) + } + var out [][]float64 + for _, row := range result.Rows() { + var reals []float64 + for _, value := range row.Cells()[position].Values() { + real, ok := value.Real() + if !ok { + t.Fatalf("%s cell = %+v, want reals", column, row.Cells()[position].Values()) + } + reals = append(reals, real) + } + out = append(out, reals) } + return out } func TestExecuteComputedMetaclassFeatureReadsDeclaration(t *testing.T) { @@ -426,7 +447,9 @@ calc def Variations :> Query { } } -func TestExecuteComputedMultiValuedFeatureIsTyped(t *testing.T) { +// A column reading a `[0..*]` feature holds the row's values in declaration +// order, and is empty for a row that binds none. +func TestExecuteComputedMultiValuedFeatureFillsCell(t *testing.T) { fixture := loadExecutionFixture(t, ` part def Box { attribute sizes : Real[0..*]; @@ -435,13 +458,45 @@ part shed { part b : Box { attribute redefines sizes = (1.0, 2.0); } + part c : Box; } -calc def Bad :> Query { +calc def Sizes :> Query { in root : Element; Project( source = Descendants(source = root, maxDepth = 1), + properties = ("name"), columns = (Column(name = "s", expression = Box::sizes)) ) +}`) + result, err := fixture.execute(t, "Sizes", Bindings{ + "root": {ElementValue(fixture.symbol(t, "shed"))}, + }, Options{}) + if err != nil { + t.Fatalf("execute: %v", err) + } + if got := cellReals(t, result, "s"); !slices.EqualFunc(got, [][]float64{{1, 2}, nil}, slices.Equal) { + t.Fatalf("s cells = %v, want both sizes for b and an empty cell for c", got) + } +} + +// A feature declared without a multiplicity holds exactly one value, so a row +// binding two fails the column with the declared bound in the error. +func TestExecuteComputedScalarFeatureRejectsSeveralValues(t *testing.T) { + fixture := loadExecutionFixture(t, ` +part def Box { + attribute size : Real; +} +part shed { + part b : Box { + attribute redefines size = (1.0, 2.0); + } +} +calc def Bad :> Query { + in root : Element; + Project( + source = Descendants(source = root, maxDepth = 1), + columns = (Column(name = "s", expression = Box::size)) + ) }`) _, err := fixture.execute(t, "Bad", Bindings{ "root": {ElementValue(fixture.symbol(t, "shed"))}, @@ -450,12 +505,43 @@ calc def Bad :> Query { if !errors.As(err, &executionError) || executionError.Kind != ErrorColumnCardinality { t.Fatalf("error = %v", err) } - if executionError.Property != "s" || executionError.Actual != "2" { + if executionError.Property != "s" || executionError.Actual != "2" || executionError.Expected != "[1..1]" { t.Fatalf("error provenance = %+v", executionError) } if !executionError.Origin.Located() { t.Fatal("cardinality errors must retain source provenance") } + if !strings.Contains(err.Error(), "produced 2 values") || !strings.Contains(err.Error(), "[1..1]") { + t.Fatalf("error text = %q, want the count and the declared multiplicity", err) + } +} + +// A `[1]` feature a row leaves unbound is absent, not an empty cell. +func TestExecuteComputedRequiredFeatureRejectsNoValue(t *testing.T) { + fixture := loadExecutionFixture(t, ` +part def Box { + attribute size : Real[1]; +} +part shed { + part b : Box; +} +calc def Bad :> Query { + in root : Element; + Project( + source = Descendants(source = root, maxDepth = 1), + columns = (Column(name = "s", expression = Box::size)) + ) +}`) + _, err := fixture.execute(t, "Bad", Bindings{ + "root": {ElementValue(fixture.symbol(t, "shed"))}, + }, Options{}) + var executionError *Error + if !errors.As(err, &executionError) || executionError.Kind != ErrorColumnAbsent { + t.Fatalf("error = %v, want %v", err, ErrorColumnAbsent) + } + if executionError.Property != "s" { + t.Fatalf("error provenance = %+v", executionError) + } } func TestExecuteComputedUserFeatureNamedLikeMetadata(t *testing.T) { diff --git a/internal/doc/queryexec/documentation_test.go b/internal/doc/queryexec/documentation_test.go index ab5e84dbeb..b33a658773 100644 --- a/internal/doc/queryexec/documentation_test.go +++ b/internal/doc/queryexec/documentation_test.go @@ -1,9 +1,7 @@ package queryexec import ( - "errors" "slices" - "strings" "testing" ) @@ -278,10 +276,10 @@ calc def Q :> Query { } } -// A computed column is one value per row, so an element with two doc bodies -// fails the column the way every multi-valued feature does. -func TestExecuteComputedDocumentationReportsSeveralBodies(t *testing.T) { - fixture := loadExecutionFixture(t, documentedBody+` +// Documentation is a `[0..*]` feature, so an element with two doc bodies fills +// the column with both, in declaration order, and `??` still defaults absence. +func TestExecuteComputedDocumentationHoldsSeveralBodies(t *testing.T) { + result := documentedRows(t, ` calc def Q :> Query { in root : Element; Project( @@ -290,14 +288,13 @@ calc def Q :> Query { columns = (Column(name = "text", expression = Element::documentation ?? "undocumented")) ) }`) - _, err := fixture.execute(t, "Q", Bindings{ - "root": {ElementValue(fixture.symbol(t, "spec"))}, - }, Options{}) - var executionError *Error - if !errors.As(err, &executionError) || executionError.Kind != ErrorColumnCardinality { - t.Fatalf("error = %v, want %v", err, ErrorColumnCardinality) + want := [][]string{ + {"The mission shall safely return\nall three crew members to Earth."}, + {"The mission shall achieve a soft landing on the lunar surface."}, + {"undocumented"}, + {"Short form.", "Long form."}, } - if executionError.Property != "text" || !strings.HasSuffix(executionError.Target, "TwoBodies") { - t.Fatalf("error = %+v, want the text column of TwoBodies", executionError) + if got := cellStrings(t, result, "text"); !slices.EqualFunc(got, want, slices.Equal) { + t.Fatalf("text cells = %q, want %q", got, want) } } diff --git a/internal/doc/queryexec/errors.go b/internal/doc/queryexec/errors.go index fcd6f2d9b7..e792c01a41 100644 --- a/internal/doc/queryexec/errors.go +++ b/internal/doc/queryexec/errors.go @@ -206,11 +206,12 @@ func (e *Error) columnMessage() (string, bool) { ), true case ErrorColumnCardinality: return fmt.Sprintf( - "query %s column %s produced %s values, expected one for %s", + "query %s column %s produced %s values for %s, outside its declared multiplicity %s", e.Query, e.Property, e.Actual, e.Target, + e.Expected, ), true case ErrorColumnDivisionByZero: return fmt.Sprintf("query %s column %s divides by zero for %s", e.Query, e.Property, e.Target), true diff --git a/internal/ir/queryplan/columns.go b/internal/ir/queryplan/columns.go index 91178aad42..7a1ac1af79 100644 --- a/internal/ir/queryplan/columns.go +++ b/internal/ir/queryplan/columns.go @@ -251,17 +251,19 @@ func (c *compiler) compileColumnReference( for _, param := range c.model.BehaviorParametersOf(query) { if !param.IsResult && c.parameterIncludes(param.Symbol, target) { return Expression{ - operation: OperationParameter, - target: param.Symbol.Name, - origin: symbols.NodeOrigin(owner.DocName, expression), + operation: OperationParameter, + target: param.Symbol.Name, + multiplicity: c.parameterMultiplicity(param.Symbol), + origin: symbols.NodeOrigin(owner.DocName, expression), }, c.staticPrimType(param.Symbol), nil } } return Expression{ - operation: OperationRowProperty, - target: target.Name, - value: declaringTypeFQN(target), - origin: symbols.NodeOrigin(owner.DocName, expression), + operation: OperationRowProperty, + target: target.Name, + value: declaringTypeFQN(target), + multiplicity: c.featureMultiplicity(target), + origin: symbols.NodeOrigin(owner.DocName, expression), }, c.staticPrimType(target), nil } diff --git a/internal/ir/queryplan/compiler.go b/internal/ir/queryplan/compiler.go index 5a92a148ce..310a3dee20 100644 --- a/internal/ir/queryplan/compiler.go +++ b/internal/ir/queryplan/compiler.go @@ -317,8 +317,8 @@ func (c *compiler) validateDefault( Kind: ErrorDefaultMultiplicity, Query: symbols.FQNOf(query), Parameter: param.Name, - Expected: multiplicityString(param.Multiplicity), - Actual: multiplicityString(value.multiplicity), + Expected: param.Multiplicity.String(), + Actual: value.multiplicity.String(), Origin: origin, } } @@ -406,6 +406,15 @@ func (c *compiler) parameterMultiplicity(sym *symbols.Symbol) Multiplicity { break } } + return multiplicityOf(rng) +} + +// featureMultiplicity is the multiplicity governing a feature a column reads. +func (c *compiler) featureMultiplicity(sym *symbols.Symbol) Multiplicity { + return multiplicityOf(c.model.GoverningMultiplicityOf(sym)) +} + +func multiplicityOf(rng semantics.Range) Multiplicity { return Multiplicity{ Lower: rng.Lower.Value, Upper: rng.Upper.Value, @@ -1080,8 +1089,8 @@ func (c *compiler) validateArgument( Query: symbols.FQNOf(query), Target: target, Parameter: param.Name, - Expected: multiplicityString(param.Multiplicity), - Actual: multiplicityString(value.multiplicity), + Expected: param.Multiplicity.String(), + Actual: value.multiplicity.String(), Origin: origin, } } @@ -1206,17 +1215,6 @@ func multiplicityConforms(actual, expected Multiplicity) bool { return !actual.UpperInfinite && actual.Upper <= expected.Upper } -func multiplicityString(multiplicity Multiplicity) string { - if !multiplicity.Known { - return "unknown" - } - upper := strconv.FormatInt(multiplicity.Upper, 10) - if multiplicity.UpperInfinite { - upper = "*" - } - return "[" + strconv.FormatInt(multiplicity.Lower, 10) + ".." + upper + "]" -} - func qualifiedNames(syms []*symbols.Symbol) []string { names := make([]string, len(syms)) for i, sym := range syms { diff --git a/internal/ir/queryplan/plan.go b/internal/ir/queryplan/plan.go index b2523181d1..071ba26b94 100644 --- a/internal/ir/queryplan/plan.go +++ b/internal/ir/queryplan/plan.go @@ -2,6 +2,8 @@ package queryplan import ( + "strconv" + "github.com/Open-MBEE/OpenSysML/internal/semantic/semantics" "github.com/Open-MBEE/OpenSysML/internal/semantic/symbols" ) @@ -63,7 +65,8 @@ const ( LiteralQuantity LiteralKind = "quantity" ) -// Multiplicity is a query parameter's effective cardinality. +// Multiplicity is the effective cardinality of a query parameter or of the +// feature a column reads. type Multiplicity struct { Lower int64 Upper int64 @@ -71,6 +74,28 @@ type Multiplicity struct { Known bool } +// Admits reports whether a value count lies within a known multiplicity; an +// unknown multiplicity admits every count. +func (m Multiplicity) Admits(count int) bool { + if !m.Known { + return true + } + n := int64(count) + return n >= m.Lower && (m.UpperInfinite || n <= m.Upper) +} + +// String writes the multiplicity in notation form, `[lower..upper]`. +func (m Multiplicity) String() string { + if !m.Known { + return "unknown" + } + upper := strconv.FormatInt(m.Upper, 10) + if m.UpperInfinite { + upper = "*" + } + return "[" + strconv.FormatInt(m.Lower, 10) + ".." + upper + "]" +} + // Parameter is one typed query input or result. A defaulted input carries its // compiled default and the query whose declaration supplied it. type Parameter struct { @@ -97,14 +122,15 @@ type Argument struct { // Expression is one immutable node of a compiled query plan. type Expression struct { - operation Operation - target string - literal LiteralKind - value string - quantity *semantics.Quantity - element *symbols.Symbol - arguments []Argument - origin symbols.Origin + operation Operation + target string + literal LiteralKind + value string + quantity *semantics.Quantity + element *symbols.Symbol + multiplicity Multiplicity + arguments []Argument + origin symbols.Origin } // Operation returns the operation this expression performs. @@ -130,6 +156,10 @@ func (e Expression) Element() (*symbols.Symbol, bool) { return e.element, e.operation == OperationElement && e.element != nil } +// Multiplicity returns the declared multiplicity of the feature or parameter +// a column expression reads, which bounds how many values a cell may hold. +func (e Expression) Multiplicity() Multiplicity { return e.multiplicity } + // Arguments returns an independent copy of the expression's arguments. func (e Expression) Arguments() []Argument { out := make([]Argument, len(e.arguments)) diff --git a/internal/semantic/semantics/collection.go b/internal/semantic/semantics/collection.go index 4c481e6c22..086b5f1091 100644 --- a/internal/semantic/semantics/collection.go +++ b/internal/semantic/semantics/collection.go @@ -120,6 +120,15 @@ func (m *Model) governingMultiplicity(sym *symbols.Symbol) (Range, bool) { return Range{}, false } +// GoverningMultiplicityOf returns the multiplicity governing a feature: the one +// it declares or inherits by redefinition, or the assumed 1..1 where it has none. +func (m *Model) GoverningMultiplicityOf(sym *symbols.Symbol) Range { + if r, ok := m.governingMultiplicity(sym); ok { + return r + } + return AssumedRange() +} + func knownRange(r Range, ok bool) (Range, bool) { return r, ok && r.Lower.Known && r.Upper.Known } From 9e6f17879e3c4c348d9b5b51975c6ed407efd06b Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 24 Sep 2026 21:06:02 +0000 Subject: [PATCH 2/7] fix(view): draw a positioned view's placed nodes in every graph form A view some DiagramLayout::Layout positions was thinned to its placed members only by the DOT writer; the Mermaid and PlantUML forms drew the whole exposed tree, so a figure exposing an unplaced package expanded into the package's contents and tripped the Mermaid workload bound. One placement classification now serves every graph form: the placed nodes and the edges between them by default, every node under UnplacedStrip, each accounted for in the form's comment syntax. Co-Authored-By: jason.han --- cmd/sysml/render.go | 2 +- cmd/sysml/usage.go | 14 +- docs/project/diagram-layout-annotations.md | 2 +- docs/project/view-rendering-forms.md | 9 +- docs/reference/cli.md | 14 +- docs/reference/sysml-v1-migration.md | 5 +- internal/doc/docpdf/docpdf.go | 4 +- internal/doc/docrender/html.go | 4 +- internal/doc/docrender/markdown.go | 4 +- internal/ir/view/dot.go | 65 +++---- internal/ir/view/form.go | 8 +- internal/ir/view/mermaid.go | 5 +- internal/ir/view/placement.go | 128 +++++++++++++ internal/ir/view/placement_test.go | 207 +++++++++++++++++++++ internal/ir/view/plantuml.go | 10 +- packaging/man/man1/sysml.1 | 19 +- 16 files changed, 424 insertions(+), 76 deletions(-) create mode 100644 internal/ir/view/placement.go create mode 100644 internal/ir/view/placement_test.go diff --git a/cmd/sysml/render.go b/cmd/sysml/render.go index add4c18a66..c457ba50ff 100644 --- a/cmd/sysml/render.go +++ b/cmd/sysml/render.go @@ -209,7 +209,7 @@ func renderOptions(width int) (view.Options, error) { } // unplacedOption is the placement -render-unplaced names for the nodes a -// positioned DOT drawing leaves unplaced, which must be one there is; none +// positioned drawing leaves unplaced, which must be one there is; none // named is the default, leaving them undrawn. func unplacedOption() (view.Unplaced, error) { if renderUnplaced == "" { diff --git a/cmd/sysml/usage.go b/cmd/sysml/usage.go index 8551626cc3..5d22bc4572 100644 --- a/cmd/sysml/usage.go +++ b/cmd/sysml/usage.go @@ -386,11 +386,13 @@ func doc() usage.Doc { "fills their nodes by keyword family from a colourblind-safe palette " + "(okabe-ito, tol-bright, tol-muted, tol-light, brewer-set2, brewer-dark2, " + "viridis or cividis), keeping black text legible on every fill. " + - "A DOT drawing of a view whose members carry DiagramLayout positions " + - "pins each at its stated place and leaves a member with no position " + - "undrawn, so nothing lands on a positioned box; -render-unplaced strip " + - "draws those members instead, in rows in a strip below the drawing. " + - "The same setting shapes the DOT diagrams of -render-document and " + + "A view whose members carry DiagramLayout positions draws the placed " + + "members and the edges between them in every graph form, and leaves a " + + "member with no position undrawn: the DOT form pins each at its stated " + + "place, so nothing lands on a positioned box, while Mermaid and PlantUML " + + "lay the same members out themselves. -render-unplaced strip draws the " + + "unplaced members too, in rows in a strip below a DOT drawing. " + + "The same setting shapes the diagrams of -render-document and " + "-render-documents.", }, }, { @@ -594,7 +596,7 @@ func registerFlags(fs *flag.FlagSet) { fs.StringVar(&renderAllDir, "render-all", "", "Render every declared view into this directory") fs.StringVar(&renderForm, "render-form", "", "Form -render or -render-all writes: text, mermaid, markdown, dot or plantuml; default from the destination for -render, each kind's machine form for -render-all") fs.StringVar(&renderPalette, "render-palette", "", "Palette the dot or plantuml form fills nodes from, by keyword family: okabe-ito, tol-bright, tol-muted, tol-light, brewer-set2, brewer-dark2, viridis or cividis; default black and white") - fs.StringVar(&renderUnplaced, "render-unplaced", "", "Where the dot form of a view some Layout positions puts the nodes none does: omit (default) leaves them undrawn, strip draws them in rows below the drawing; applies to -render, -render-all and document diagrams") + fs.StringVar(&renderUnplaced, "render-unplaced", "", "Where a graph form of a view some Layout positions puts the nodes none does: omit (default) leaves them undrawn in every form, strip draws them, in rows below the dot drawing; applies to -render, -render-all and document diagrams") fs.StringVar(&renderDoc, "render-document", "", "Compile this document definition, run its queries and write the rendered document") fs.StringVar(&renderDocsDir, "render-documents", "", "Render every document definition, linked to one another, into this directory") diff --git a/docs/project/diagram-layout-annotations.md b/docs/project/diagram-layout-annotations.md index a73ebf3a7e..7e43e4439e 100644 --- a/docs/project/diagram-layout-annotations.md +++ b/docs/project/diagram-layout-annotations.md @@ -225,7 +225,7 @@ reads a feature rather than a literal is already an error of the type tier | Form | Positions | Notes | |---|---|---| -| `mermaid` | Not representable | Written as comments after the header so a round trip through the artifact keeps them: `%% canvas: unit=px w=1200 h=800`, `%% layout: n1 x=120 y=80 w=200 h=90 collapsed`, `%% route: n1->n2 320,125 400,125 480,125`. Node ids are the ones the diagram body uses. | +| `mermaid` | Not representable | Written as comments after the header so a round trip through the artifact keeps them: `%% canvas: unit=px w=1200 h=800`, `%% layout: n1 x=120 y=80 w=200 h=90 collapsed`, `%% route: n1->n2 320,125 400,125 480,125`. Node ids are the ones the diagram body uses. The nodes drawn are the ones the `dot` form draws: in a rendering that positions some nodes, the placed ones and the edges between them, with the unplaced counted in a `%% not represented:` notice, or every node under `Options.Unplaced = UnplacedStrip`; the `plantuml` form does the same under `' not represented:`. | | `text` | Not representable | `at (120, 80)` after a positioned node, `size 200×90` and `collapsed` when stated; `via (320, 125) (400, 125)` after a routed edge; a `canvas size … in px` line under the title. | | `dot` | Honored | Graphviz's own vocabulary, converted from y-down pixels to y-up points (`inputscale=72`, `dpi=72`; y measured from the canvas's bottom edge, negated with no canvas height): a node pinned at the centre of its box with `pos="x,y!"`, `pin=true`, `width`/`height` in inches — `fixedsize=true` for a stated size, fitted to the label for an unstated one — and `comment="collapsed"`; a cluster's `bb` stated (the stated box, or the one round its positioned members) and its anchor pinned at the centre; a route as a `pos` spline through the waypoints, a route of one waypoint noticed, as is a route `neato` redraws; the canvas echoed as `// canvas:` and held by an invisible point pinned at each corner, so the drawing's bounding box is the canvas. The `// layout:` header names `neato -n2` when every node is placed and any edge routed, `neato -n` when every node is placed and none routed, `neato` when some nodes are, `dot` when none — see [view rendering forms](view-rendering-forms.md#geometry). | | `markdown` (table) | n/a | — | diff --git a/docs/project/view-rendering-forms.md b/docs/project/view-rendering-forms.md index dc3a651355..a11f2390dd 100644 --- a/docs/project/view-rendering-forms.md +++ b/docs/project/view-rendering-forms.md @@ -411,7 +411,14 @@ digraph "PlantViews::placedView" { keeps its place in the text — a member of a positioned cluster is written in that cluster, whose stated box is not stretched to it — so Graphviz draws it below the box it belongs to. A drawing with no positioned node is unchanged by either setting. An `Unplaced` that is - neither is refused (`UnknownUnplacedError`). + neither is refused (`UnknownUnplacedError`). The classification is one `placement` + (`placement.go`) every graph-shaped form draws by: the Mermaid and PlantUML forms of a partly + positioned rendering draw the placed nodes and the edges between them, an omitted tree node's + placed members detached from the node above as DOT draws them, under a `%% not represented:` or + `' not represented:` notice with DOT's wording; under `UnplacedStrip` they draw every node, + laid out by the tool that draws them, and say so. So the three forms draw one node set and one + edge set of a positioned view, and a view exposing a package its layout does not place does not + become a chart of the package's whole contents in Mermaid. - **Engine.** The `// layout:` header names the command that honours what is written: `neato -n2` when any edge is routed (the pinned nodes and the written routes are taken as given, the other edges are drawn), `neato -n` when no edge is routed, `dot` when no node is diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 74cf809295..2398a270a0 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -221,7 +221,7 @@ reported, so a script that reads it takes the output from the first `{`. | `--render-all ` | | Render every declared view into the directory, one artifact per view | | `--render-form
` | | Form `--render` or `--render-all` writes: `text`, `mermaid`, `markdown`, `dot` or `plantuml` (default: destination-dependent for `--render`, each kind's machine-readable form for `--render-all`) | | `--render-palette ` | | Palette the `dot` or `plantuml` form of `--render` or `--render-all` fills nodes with, by keyword family: `okabe-ito`, `tol-bright`, `tol-muted`, `tol-light`, `brewer-set2`, `brewer-dark2`, `viridis` or `cividis`; black and white when absent. Mermaid notes it as not represented; text and Markdown ignore it. An unknown name is refused with the names there are (see [Rendering a view](#rendering-a-view)) | -| `--render-unplaced ` | | Where the `dot` form of a view some `DiagramLayout::Layout` positions puts the nodes none does: `omit` (the default) leaves them, and the edges at them, undrawn; `strip` draws them in rows below the drawing, clear of the canvas and every positioned box. Applies to `--render`, `--render-all` and the `dot` diagrams of `--render-document` and `--render-documents`; a view with no positioned node is laid out as before whichever is named. An unknown placement is refused with the placements there are (see [Rendering a view](#rendering-a-view)) | +| `--render-unplaced ` | | Where a graph form of a view some `DiagramLayout::Layout` positions puts the nodes none does: `omit` (the default) leaves them, and the edges at them, undrawn in every form, so the `mermaid`, `dot` and `plantuml` forms draw one node set; `strip` draws them too, in rows below the `dot` drawing, clear of the canvas and every positioned box, and among the placed nodes in the forms that lay nodes out themselves. Applies to `--render`, `--render-all` and the diagrams of `--render-document` and `--render-documents`; a view with no positioned node is laid out as before whichever is named. An unknown placement is refused with the placements there are (see [Rendering a view](#rendering-a-view)) | | `--render-document ` | | Compile a document definition (a `part def` specializing `DocumentQueries::Document`), run its queries against the model, render its diagram blocks through the view engine and write the result as CommonMark Markdown, as `%render-document` does. Paragraphs may hold inline runs (`Span` with a `plain`/`emphasis`/`strong`/`code` style, `Link` to a URL, `Ref` linking to another content block's anchor); a query-backed paragraph or list styles its projected values through nested `SpanColumn`/`LinkColumn` column runs; a table with a `groupBy` column writes one subtable per group value, with the query's projected properties and computed `Column` names as its columns. A `Diagram` block embeds a declared view, or an element with a stated rendering kind, as a fenced ` ```mermaid ` block (a fenced ` ```dot ` block of Graphviz DOT under `-diagram-form dot`, a ` ```plantuml ` block under `-diagram-form plantuml`; a table-kind view as a pipe table whichever form), with an optional caption and `TB`/`LR`/`RL`/`BT` flow direction. Markdown is the default form; `-doc-form html` renders the same document tree as semantic HTML (see [Rendering a document as HTML](#rendering-a-document-as-html)) and `-doc-form pdf` converts the Markdown (see [Rendering a document as PDF](#rendering-a-document-as-pdf)). Combined with `--instantiate`, the document's queries run over the objects created (see [Rendering a document over objects](#rendering-a-document-over-objects)). `-json` does not apply. See the [document generation manual](../manual/README.md) | | `--doc-form ` | | Form `--render-document` writes: `markdown` (default), `html`, rendered from the document tree itself (see [Rendering a document as HTML](#rendering-a-document-as-html)), or `pdf`, which drives an external converter | | `--diagram-form ` | | Form the graph-shaped diagram blocks of `--render-document` and `--render-documents` are written in: `mermaid` (default), `dot`, Graphviz DOT for a toolchain that lays diagrams out with Graphviz, produced without Graphviz installed, or `plantuml`, PlantUML in the Pilot visualizer's B&W style, produced without a PlantUML jar. Applies to every diagram of the document in every `--doc-form`; a table-kind view is a table whichever form, and a `sequence` diagram, which has no DOT form, is refused under `dot` | @@ -608,7 +608,10 @@ line is the only one printed. `TB`/`LR` become `top to bottom direction`/`left t PlantUML has no reversed direction, so `BT`/`RL` take the nearest forward one under a `' not represented:` notice. PlantUML pins no position either, so DiagramLayout geometry is kept as `' canvas:`, `' layout:` and `' route:` comments and noticed — `-render-form dot` is the form that -honours it ([the PlantUML section](../project/view-rendering-forms.md#plantuml)). Producing PlantUML +honours it ([the PlantUML section](../project/view-rendering-forms.md#plantuml)) — but the diagram +draws the nodes the DOT form draws: in a view that positions some nodes, the placed ones and the +edges between them, with the unplaced accounted for in a `' not represented:` notice, and every +node under `-render-unplaced strip`. Producing PlantUML needs no Java and no PlantUML jar; drawing the file does (`java -jar plantuml.jar -tsvg view.puml`). `-render-palette ` fills the DOT and PlantUML nodes with a colourblind-safe palette by **keyword @@ -657,7 +660,12 @@ the edges at it — a migrated diagram shows what its source showed, and nothing placed box — and a `// not represented:` notice counts what was left out; `-render-unplaced strip` draws those nodes instead, in rows below the canvas or the positioned boxes, wrapped at the drawing's width and clear of it and of one another. Either way every node -drawn is pinned, so `neato` is never left to place one. +drawn is pinned, so `neato` is never left to place one. The Mermaid and PlantUML forms draw the +same node set and edge set: the placed nodes alone by default, under a `%% not represented:` or +`' not represented:` notice counting the unplaced, and every node under `-render-unplaced strip`, +laid out by the tool that draws them since neither pins a position. So a positioned view's figure +in a document shows the picture its layout describes in whichever `-diagram-form`, and an +exposed package the layout does not place does not expand into a chart of its whole contents. A model with no layout annotations renders exactly as before. `-validate` reports a `Layout` or `Route` on an element the rendering does not draw as a node or an edge, a `Route` with an odd number of values, a `Canvas` outside a view, and two positions for one element in one view (the diff --git a/docs/reference/sysml-v1-migration.md b/docs/reference/sysml-v1-migration.md index 32b9eaa6ef..1d393aabd4 100644 --- a/docs/reference/sysml-v1-migration.md +++ b/docs/reference/sysml-v1-migration.md @@ -642,7 +642,10 @@ and refuses the three, so `-doc-form markdown` writes the title as its first hea section tree as nested headings, unnumbered. The rendered figures are the migrated views: a block or internal block diagram drawn from its exposures, an activity or state machine diagram drawn from its graph, each positioned where the MTIP layout put it when `-layout` was given -(see [Layout from an MTIP export](#layout-from-an-mtip-export)); a diagram the +(see [Layout from an MTIP export](#layout-from-an-mtip-export)) — and showing, in every +`-diagram-form`, the elements the layout placed: an exposed package the source diagram did not +draw stays out of the figure rather than expanding into its whole contents, and +`-render-unplaced strip` adds the unplaced elements; a diagram the migration left out of the document (empty, or rendered as textual notation) is absent from the render and the report says why, so a rendered document holds no empty figure. Styling beyond what the model carries — a cover image, a tool's fonts, its header and footer — is not diff --git a/internal/doc/docpdf/docpdf.go b/internal/doc/docpdf/docpdf.go index 53b4ebbdeb..51b1949901 100644 --- a/internal/doc/docpdf/docpdf.go +++ b/internal/doc/docpdf/docpdf.go @@ -50,8 +50,8 @@ type Options struct { // when empty. DiagramForm view.Form - // Unplaced is where a DOT diagram some Layout positions puts the nodes - // none does: left undrawn when empty, or in a strip below the drawing. + // Unplaced is where a diagram some Layout positions puts the nodes none + // does: left undrawn when empty, or drawn too (a strip below a DOT drawing). Unplaced view.Unplaced } diff --git a/internal/doc/docrender/html.go b/internal/doc/docrender/html.go index 90d23b93d8..5f5ad95632 100644 --- a/internal/doc/docrender/html.go +++ b/internal/doc/docrender/html.go @@ -171,8 +171,8 @@ type HTMLOptions struct { // Mermaid when empty; a table-kind view is a table whichever it is. DiagramForm view.Form - // Unplaced is where a DOT diagram some Layout positions puts the nodes - // none does: left undrawn when empty, or in a strip below the drawing. + // Unplaced is where a diagram some Layout positions puts the nodes none + // does: left undrawn when empty, or drawn too (a strip below a DOT drawing). Unplaced view.Unplaced // DiagramImages are images drawn ahead of the render, one per graph-shaped diff --git a/internal/doc/docrender/markdown.go b/internal/doc/docrender/markdown.go index 3dfec44d83..1d6a7d8b07 100644 --- a/internal/doc/docrender/markdown.go +++ b/internal/doc/docrender/markdown.go @@ -24,8 +24,8 @@ type MarkdownOptions struct { // Mermaid when empty; a table-kind view is a pipe table whichever it is. DiagramForm view.Form - // Unplaced is where a DOT diagram some Layout positions puts the nodes - // none does: left undrawn when empty, or in a strip below the drawing. + // Unplaced is where a diagram some Layout positions puts the nodes none + // does: left undrawn when empty, or drawn too (a strip below a DOT drawing). Unplaced view.Unplaced } diff --git a/internal/ir/view/dot.go b/internal/ir/view/dot.go index 985a60ccbd..feec4c5c3a 100644 --- a/internal/ir/view/dot.go +++ b/internal/ir/view/dot.go @@ -60,7 +60,7 @@ func (r *Rendering) DOTWith(options Options) (string, error) { direction := options.Direction w := newDOTWriter(r, options) edges := r.Edges - if w.placed > 0 && w.placed < w.nodes { + if w.placement.partial() { edges = w.settleUnplaced(r.Roots, r.Edges, options.Unplaced) } for _, edge := range edges { @@ -125,13 +125,13 @@ func (r *Rendering) DOTWith(options Options) (string, error) { // it, the clusters and the palette's families collected. func newDOTWriter(r *Rendering, options Options) *dotWriter { w := &dotWriter{tree: r.Kind == KindTree, clusters: map[string]bool{}, enclosing: map[string][]string{}, canvas: r.Canvas, - boxes: map[string]nodeBox{}, omitted: map[string]bool{}, fills: familyFills{palette: options.Palette, tree: r.Kind == KindTree}, labels: labelsOf(r.Roots)} + placement: placeRendering(r), boxes: map[string]nodeBox{}, omitted: map[string]bool{}, + fills: familyFills{palette: options.Palette, tree: r.Kind == KindTree}, labels: labelsOf(r.Roots)} w.placeNodes(r.Roots, r.Edges) for _, root := range r.Roots { if !w.tree { w.collectClusters(root, nil) } - w.countPlaced(root) w.fills.collect(root) } return w @@ -141,32 +141,22 @@ func newDOTWriter(r *Rendering, options Options) *dotWriter { // asked: boxed in a strip below the drawing, or left undrawn with the edges // at them; either is noticed. The edges left to write are returned. func (w *dotWriter) settleUnplaced(roots []*Node, edges []Edge, unplaced Unplaced) []Edge { - count := w.nodes - w.placed + count := w.placement.unplaced() if unplaced == UnplacedStrip { w.stripUnplaced(roots, edges) - w.placed = w.nodes w.notices = append(w.notices, fmt.Sprintf("%d node(s) without a position, drawn in a strip below the drawing", count)) return edges } w.omitUnplaced(roots) - kept := make([]Edge, 0, len(edges)) - for _, edge := range edges { - if !w.omitted[edge.From] && !w.omitted[edge.To] { - kept = append(kept, edge) - } - } - notice := fmt.Sprintf("%d node(s) without a position, left undrawn", count) - if dropped := len(edges) - len(kept); dropped > 0 { - notice += fmt.Sprintf(", and %d edge(s) at them", dropped) - } - w.notices = append(w.notices, notice) + kept, dropped := w.placement.keptEdges(edges) + w.notices = append(w.notices, w.placement.omitNotice(dropped)) return kept } -// omitUnplaced marks every node under nodes that has no box as undrawn. +// omitUnplaced marks every node under nodes that has no place as undrawn. func (w *dotWriter) omitUnplaced(nodes []*Node) { for _, node := range nodes { - if _, ok := w.boxes[node.ID]; !ok { + if !w.placement.placed[node.ID] { w.omitted[node.ID] = true } w.omitUnplaced(node.Children) @@ -276,15 +266,14 @@ type dotWriter struct { enclosing map[string][]string // node ID -> the cluster IDs around it compound bool // an edge is clipped at a cluster canvas *Canvas // the surface positions are flipped against + placement *placement // which nodes have a place, shared with every form boxes map[string]nodeBox // node ID -> the box it is drawn in, for every node that has one stated map[string]bool // node IDs the drawing itself boxes, once a strip adds boxes of its own omitted map[string]bool // node IDs left undrawn for want of a box - nodes int // nodes in the rendering, and how many have a box - placed int - routed int // edges with a route to write - notices []string // geometry the form cannot draw - fills familyFills // the palette fills, by keyword family - labels labeller // the node labels, headed relative to the roots' namespace + routed int // edges with a route to write + notices []string // geometry the form cannot draw + fills familyFills // the palette fills, by keyword family + labels labeller // the node labels, headed relative to the roots' namespace } // The Standard B&W style, after the sysmlbw PlantUML skin: Helvetica text, @@ -308,17 +297,6 @@ var ( func dotColorAttr(color string) string { return "color=" + dotQuote(color) } func dotFontAttr(name string) string { return "fontname=" + dotQuote(name) } -// countPlaced counts the nodes under node and those with a box to pin them in. -func (w *dotWriter) countPlaced(node *Node) { - w.nodes++ - if _, ok := w.boxes[node.ID]; ok { - w.placed++ - } - for _, child := range node.Children { - w.countPlaced(child) - } -} - // nodeBox is where a node is drawn, top-left to bottom-right in pixels; stated // when the Layout gives its size and not only its corner. type nodeBox struct { @@ -358,12 +336,15 @@ func (w *dotWriter) placeNodes(roots []*Node, edges []Edge) { } } -// placeNode records the box of node and of the nodes under it, members first -// so a cluster can be boxed round them. +// placeNode records the box of every placed node under and including node, +// members first so a cluster can be boxed round them. func (w *dotWriter) placeNode(node *Node, ends map[string][]routeEnd) { for _, child := range node.Children { w.placeNode(child, ends) } + if !w.placement.placed[node.ID] { + return + } cluster := len(node.Children) > 0 && !w.tree switch { case node.Geometry != nil && cluster: @@ -372,7 +353,7 @@ func (w *dotWriter) placeNode(node *Node, ends map[string][]routeEnd) { w.boxes[node.ID] = w.statedBox(node) case cluster && w.membersBox(node) != nil: w.boxes[node.ID] = *w.membersBox(node) - case len(ends[node.ID]) > 0: + default: w.boxes[node.ID] = w.routedBox(node, ends[node.ID]) } } @@ -427,7 +408,7 @@ func dotReach(node *Node, width, height, ux, uy float64) float64 { // Every node drawn in a positioned drawing is pinned, so plain `neato` is never named. func (w *dotWriter) engine() string { switch { - case w.placed == 0: + case w.placement.count == 0: return "dot" case w.routed > 0: return "neato -n2" @@ -469,7 +450,7 @@ func (w *dotWriter) graphAttributes(direction Direction) []string { if w.compound { attrs = append(attrs, "compound=true") } - if w.placed > 0 { + if w.placement.count > 0 { attrs = append(attrs, "inputscale=72", "dpi=72") } return attrs @@ -482,7 +463,7 @@ var dotCanvasCorners = [2]string{"canvas:0", "canvas:1"} // canvas, so the drawing's `bb` is the canvas; it needs an engine that keeps pins. func (w *dotWriter) writeCanvas() { c := w.canvas - if c == nil || !c.HasSize || w.placed == 0 { + if c == nil || !c.HasSize || w.placement.count == 0 { return } for i, corner := range [2]Point{{}, {X: c.Width, Y: c.Height}} { @@ -1005,7 +986,7 @@ func (w *dotWriter) membersBox(node *Node) *nodeBox { var box *nodeBox for _, child := range node.Children { member, ok := w.boxes[child.ID] - if !ok || member.low == member.high { + if !ok || !w.placement.extent[child.ID] { continue } grown := nodeBox{low: Point{X: member.low.X - dotClusterMargin, Y: member.low.Y - dotClusterMargin}, diff --git a/internal/ir/view/form.go b/internal/ir/view/form.go index a6d78267f2..14872f6eb9 100644 --- a/internal/ir/view/form.go +++ b/internal/ir/view/form.go @@ -136,8 +136,8 @@ func (e *WrongFormError) Unwrap() error { return ErrWrongForm } // Options are what a rendering is written with beside its form. Each form // takes the ones that apply to it: the text form its Width, the Mermaid form -// its Direction, the DOT and PlantUML forms their Direction and Palette, the -// DOT form its Unplaced. A form ignores the rest, the Mermaid form saying so +// its Direction and Unplaced, the DOT and PlantUML forms their Direction, +// Palette and Unplaced. A form ignores the rest, the Mermaid form saying so // of a Palette in a comment. type Options struct { // Direction is the flow direction a graph-shaped form is drawn in; empty @@ -146,8 +146,8 @@ type Options struct { // Palette is the palette the DOT and PlantUML forms fill nodes from, by // keyword family; empty draws in black and white. Palette Palette - // Unplaced is what the DOT form does with the nodes a positioned drawing - // leaves unplaced; empty leaves them undrawn, as UnplacedOmit does. + // Unplaced is what a graph-shaped form does with the nodes a positioned + // drawing leaves unplaced; empty leaves them undrawn, as UnplacedOmit does. Unplaced Unplaced // Width is the width the text form is written to fit; WidthUnbounded // writes every column as wide as its widest cell. diff --git a/internal/ir/view/mermaid.go b/internal/ir/view/mermaid.go index b1bdfbe770..48cbc262bd 100644 --- a/internal/ir/view/mermaid.go +++ b/internal/ir/view/mermaid.go @@ -25,8 +25,11 @@ func (r *Rendering) Mermaid() string { // stated direction: a flowchart flows that way, and a state diagram states it // as a `direction` statement. The empty direction keeps each kind's default, // and a kind no direction applies to ignores it. A palette is not drawn, -// Mermaid having no fill per node kind, and is noted as not represented. +// Mermaid having no fill per node kind, and is noted as not represented. A +// rendering some Layout positions draws the nodes the DOT form draws: the placed +// ones, and the unplaced ones too under UnplacedStrip. func (r *Rendering) MermaidWith(options Options) string { + r = r.settleUnplaced(options.Unplaced, FormMermaid) direction := options.Direction var b strings.Builder labels := labelsOf(r.Roots) diff --git a/internal/ir/view/placement.go b/internal/ir/view/placement.go new file mode 100644 index 0000000000..73de40a759 --- /dev/null +++ b/internal/ir/view/placement.go @@ -0,0 +1,128 @@ +package view + +import "fmt" + +// placement is what a positioned rendering has a place for: the nodes a Layout +// positions, the clusters round a placed member, and the nodes a route meets. +// Every graph-shaped form draws by it, so a drawing shows the same nodes and +// edges whichever form writes it. +type placement struct { + placed map[string]bool // node ID -> has a place + extent map[string]bool // node ID -> its place has an extent, not a corner alone + nodes int // nodes in the rendering, and how many have a place + count int +} + +// placeRendering classifies every node of r. A rendering no Layout or Route +// positions places nothing, and every form draws all of it. +func placeRendering(r *Rendering) *placement { + p := &placement{placed: map[string]bool{}, extent: map[string]bool{}} + routed := map[string]bool{} + for _, edge := range r.Edges { + if len(edge.Route) > 1 { + routed[edge.From], routed[edge.To] = true, true + } + } + for _, root := range r.Roots { + p.place(root, r.Kind == KindTree, routed) + } + return p +} + +// place classifies node and the nodes under it, members first so a cluster +// can take its place from them. A cluster a Layout gives a corner but no size +// has a place without an extent while no member has one. +func (p *placement) place(node *Node, tree bool, routed map[string]bool) { + p.nodes++ + members := false + for _, child := range node.Children { + p.place(child, tree, routed) + members = members || p.extent[child.ID] + } + cluster := len(node.Children) > 0 && !tree + switch { + case node.Geometry != nil: + p.extent[node.ID] = !cluster || node.Geometry.HasSize || members + case cluster && members, routed[node.ID]: + p.extent[node.ID] = true + default: + return + } + p.placed[node.ID] = true + p.count++ +} + +// partial reports whether the rendering places some nodes and not others: the +// case a form settles as its Options.Unplaced asks. +func (p *placement) partial() bool { return p.count > 0 && p.count < p.nodes } + +// unplaced is how many nodes have no place. +func (p *placement) unplaced() int { return p.nodes - p.count } + +// omit is r without the nodes it leaves unplaced and the edges at them, with a +// notice of what was left undrawn. In a tree an omitted node's placed members +// become roots of their own, detached from the node above; a cluster round a +// placed member has a place itself, so a clustered kind omits whole subtrees. +func (p *placement) omit(r *Rendering) *Rendering { + out := *r + var hoisted []*Node + out.Roots = append(p.keep(r.Roots, &hoisted), hoisted...) + kept, dropped := p.keptEdges(r.Edges) + out.Edges = kept + out.Notices = append(append([]string(nil), r.Notices...), p.omitNotice(dropped)) + return &out +} + +// keptEdges is edges without those at an unplaced node, and how many those were. +func (p *placement) keptEdges(edges []Edge) (kept []Edge, dropped int) { + kept = make([]Edge, 0, len(edges)) + for _, edge := range edges { + if p.placed[edge.From] && p.placed[edge.To] { + kept = append(kept, edge) + } + } + return kept, len(edges) - len(kept) +} + +// omitNotice accounts for the unplaced nodes left undrawn, and the edges at them. +func (p *placement) omitNotice(dropped int) string { + notice := fmt.Sprintf("%d node(s) without a position, left undrawn", p.unplaced()) + if dropped > 0 { + notice += fmt.Sprintf(", and %d edge(s) at them", dropped) + } + return notice +} + +// keep is nodes without the unplaced ones, collecting the kept members of an +// omitted node in hoisted. +func (p *placement) keep(nodes []*Node, hoisted *[]*Node) []*Node { + var kept []*Node + for _, node := range nodes { + members := p.keep(node.Children, hoisted) + if !p.placed[node.ID] { + *hoisted = append(*hoisted, members...) + continue + } + copied := *node + copied.Children = members + kept = append(kept, &copied) + } + return kept +} + +// settleUnplaced is what a form that lays nodes out itself draws of a +// positioned rendering: the placed nodes alone by default, every node under +// UnplacedStrip, each noticed. A rendering placing all or none is drawn whole. +func (r *Rendering) settleUnplaced(unplaced Unplaced, form Form) *Rendering { + p := placeRendering(r) + if !p.partial() { + return r + } + if unplaced != UnplacedStrip { + return p.omit(r) + } + out := *r + out.Notices = append(append([]string(nil), r.Notices...), + fmt.Sprintf("%d node(s) without a position, drawn among the placed ones; the %s form lays every node out itself", p.unplaced(), form)) + return &out +} diff --git a/internal/ir/view/placement_test.go b/internal/ir/view/placement_test.go new file mode 100644 index 0000000000..da595eec6c --- /dev/null +++ b/internal/ir/view/placement_test.go @@ -0,0 +1,207 @@ +package view + +import ( + "errors" + "regexp" + "slices" + "strings" + "testing" +) + +// partlyPlaced is an interconnection some Layouts position: two placed parts +// joined by a routed connection, a loose part, a wide definition and a group +// with two members none of which has a place. +func partlyPlaced() *Rendering { + return &Rendering{ + View: "V", + Kind: KindInterconnection, + Canvas: &Canvas{Unit: "px", Width: 300, Height: 100, HasSize: true}, + Roots: []*Node{ + {ID: "placed", Kind: "part", Name: "pump", Geometry: &Geometry{X: 10, Y: 10, Width: 100, Height: 40, HasSize: true}}, + {ID: "low", Kind: "part", Name: "tank", Geometry: &Geometry{X: 150, Y: 120, Width: 60, Height: 30, HasSize: true}}, + {ID: "loose", Kind: "part", Name: "spare"}, + {ID: "wide", Kind: "part def", Name: "A rather long definition name"}, + {ID: "group", Kind: "part def", Name: "Group", Children: []*Node{ + {ID: "g1", Kind: "part", Name: "g1"}, + {ID: "g2", Kind: "part", Name: "g2"}, + }}, + }, + Edges: []Edge{ + {From: "placed", To: "low", Kind: EdgeConnection, Route: []Point{{X: 60, Y: 50}, {X: 60, Y: 170}, {X: 150, Y: 170}}}, + {From: "placed", To: "loose", Kind: EdgeConnection}, + {From: "loose", To: "g1", Kind: EdgeFlow}, + }, + } +} + +// drawnNodes are the node IDs a written diagram declares, in order: the +// identifiers before `[` in Mermaid, after `as` in PlantUML, before `[` at the +// head of a statement in DOT. +func drawnNodes(t *testing.T, form Form, artifact string) []string { + t.Helper() + var pattern *regexp.Regexp + switch form { + case FormMermaid: + pattern = regexp.MustCompile(`(?m)^\s*(?:subgraph )?(\w+) ?\["`) + case FormPlantUML: + pattern = regexp.MustCompile(`(?m)^\s*(?:class|rectangle|state|participant) ".*" as (\w+)`) + case FormDot: + pattern = regexp.MustCompile(`(?m)^\s*"(\w+)" \[`) + default: + t.Fatalf("no node pattern for the %s form", form) + } + var ids []string + for _, match := range pattern.FindAllStringSubmatch(artifact, -1) { + ids = append(ids, match[1]) + } + slices.Sort(ids) + return ids +} + +// Every graph-shaped form of a partly placed rendering draws the placed nodes +// and the edges between them, and no other, accounting for the rest in its own +// comment syntax; UnplacedStrip draws every node in every form. +func TestGraphFormsDrawOnePlacement(t *testing.T) { + dot, err := partlyPlaced().DOT() + if err != nil { + t.Fatalf("DOT: %v", err) + } + placed := drawnNodes(t, FormDot, dot) + if !slices.Equal(placed, []string{"low", "placed"}) { + t.Fatalf("DOT draws %v of the partly placed rendering:\n%s", placed, dot) + } + notice := "not represented: 5 node(s) without a position, left undrawn, and 2 edge(s) at them\n" + cases := []struct { + form Form + comment string + edge string + }{ + {FormMermaid, "%% ", " placed --- low\n"}, + {FormPlantUML, "' ", "placed -[thickness=3]- low\n"}, + } + for _, tc := range cases { + artifact, err := partlyPlaced().Write(tc.form) + if err != nil { + t.Fatalf("%s: %v", tc.form, err) + } + if drawn := drawnNodes(t, tc.form, artifact); !slices.Equal(drawn, placed) { + t.Errorf("%s draws %v, DOT %v:\n%s", tc.form, drawn, placed, artifact) + } + if !strings.Contains(artifact, tc.comment+notice) { + t.Errorf("%s does not account for the unplaced nodes:\n%s", tc.form, artifact) + } + if !strings.Contains(artifact, tc.edge) || strings.Contains(artifact, "loose") || strings.Contains(artifact, "g1") { + t.Errorf("%s edges are not those between placed nodes:\n%s", tc.form, artifact) + } + stripped, err := partlyPlaced().WriteWith(tc.form, Options{Unplaced: UnplacedStrip}) + if err != nil { + t.Fatalf("%s strip: %v", tc.form, err) + } + all := []string{"g1", "g2", "group", "loose", "low", "placed", "wide"} + if drawn := drawnNodes(t, tc.form, stripped); !slices.Equal(drawn, all) { + t.Errorf("%s under strip draws %v, not every node:\n%s", tc.form, drawn, stripped) + } + if !strings.Contains(stripped, tc.comment+"not represented: 5 node(s) without a position, drawn among the placed ones; the "+string(tc.form)+" form lays every node out itself\n") { + t.Errorf("%s under strip does not account for the unplaced nodes:\n%s", tc.form, stripped) + } + if strings.Count(stripped, " --- ")+strings.Count(stripped, "-[thickness=3]-")+strings.Count(stripped, " -.-> ")+strings.Count(stripped, "-[dashed]->") != 3 { + t.Errorf("%s under strip does not draw every edge:\n%s", tc.form, stripped) + } + } + checkPlantUMLSyntax(t, written(t, partlyPlaced(), FormPlantUML)) +} + +// A rendering placing every node, or none, is drawn whole with no accounting. +func TestGraphFormsDrawWhollyPlacedOrUnplaced(t *testing.T) { + whole := partlyPlaced() + whole.Roots = whole.Roots[:2] + whole.Edges = whole.Edges[:1] + none := partlyPlaced() + for _, root := range none.Roots { + root.Geometry = nil + } + none.Edges[0].Route = nil + for _, r := range []*Rendering{whole, none} { + for _, form := range []Form{FormMermaid, FormPlantUML, FormDot} { + artifact, err := r.Write(form) + if err != nil { + t.Fatalf("%s: %v", form, err) + } + if strings.Contains(artifact, "without a position") { + t.Errorf("%s accounts for unplaced nodes where there is no partial placement:\n%s", form, artifact) + } + if want := len(r.Roots) + len(r.Roots[len(r.Roots)-1].Children); len(drawnNodes(t, form, artifact)) != want { + t.Errorf("%s draws %v, not all %d nodes:\n%s", form, drawnNodes(t, form, artifact), want, artifact) + } + } + } +} + +// In a tree, an unplaced node's placed members are drawn detached from the node +// above, as the DOT form draws them; in a clustered kind the node round a +// placed member has a place of its own, so the nesting is kept whole. +func TestOmittedNodeMembersTakeItsPlace(t *testing.T) { + tree := &Rendering{ + View: "V", + Kind: KindTree, + Roots: []*Node{ + {ID: "root", Kind: "package", Name: "Root", Geometry: &Geometry{X: 0, Y: 0, Width: 80, Height: 40, HasSize: true}, Children: []*Node{ + {ID: "mid", Kind: "package", Name: "Mid", Children: []*Node{ + {ID: "leaf", Kind: "part def", Name: "Leaf", Geometry: &Geometry{X: 0, Y: 100, Width: 80, Height: 40, HasSize: true}}, + }}, + }}, + }, + } + mermaid := tree.Mermaid() + if !strings.Contains(mermaid, "%% not represented: 1 node(s) without a position, left undrawn\n") { + t.Errorf("tree Mermaid header:\n%s", mermaid) + } + if !slices.Equal(drawnNodes(t, FormMermaid, mermaid), []string{"leaf", "root"}) || strings.Contains(mermaid, " --- ") { + t.Errorf("tree Mermaid draws the placed nodes detached:\n%s", mermaid) + } + dot := written(t, tree, FormDot) + if !slices.Equal(drawnNodes(t, FormDot, dot), []string{"leaf", "root"}) || strings.Contains(dot, " -> ") { + t.Errorf("tree DOT draws the placed nodes detached:\n%s", dot) + } + + clustered := &Rendering{ + View: "V", + Kind: KindInterconnection, + Roots: []*Node{ + {ID: "outer", Kind: "part", Name: "outer", Geometry: &Geometry{X: 0, Y: 0, Width: 300, Height: 300, HasSize: true}, Children: []*Node{ + {ID: "inner", Kind: "part", Name: "inner", Children: []*Node{ + {ID: "deep", Kind: "part", Name: "deep", Geometry: &Geometry{X: 20, Y: 20, Width: 80, Height: 40, HasSize: true}}, + }}, + }}, + }, + } + for _, form := range []Form{FormMermaid, FormPlantUML, FormDot} { + artifact := written(t, clustered, form) + if drawn := drawnNodes(t, form, artifact); !slices.Equal(drawn, []string{"deep", "inner", "outer"}) { + t.Errorf("%s draws %v of a cluster placed by its member:\n%s", form, drawn, artifact) + } + if strings.Contains(artifact, "without a position") { + t.Errorf("%s accounts for an unplaced node where a cluster is placed by its member:\n%s", form, artifact) + } + } +} + +// A placement asked for by a name none has is refused by every form that +// returns an error, as the DOT form refuses it. +func TestPlantUMLRefusesUnknownUnplaced(t *testing.T) { + _, err := partlyPlaced().PlantUMLWith(Options{Unplaced: "hide"}) + var unknown *UnknownUnplacedError + if !errors.As(err, &unknown) || unknown.Name != "hide" { + t.Fatalf("PlantUML with an unknown placement: %v", err) + } +} + +// written is r in form, failing the test when the form refuses it. +func written(t *testing.T, r *Rendering, form Form) string { + t.Helper() + artifact, err := r.Write(form) + if err != nil { + t.Fatalf("%s: %v", form, err) + } + return artifact +} diff --git a/internal/ir/view/plantuml.go b/internal/ir/view/plantuml.go index b1270b9ba9..bc18943c1c 100644 --- a/internal/ir/view/plantuml.go +++ b/internal/ir/view/plantuml.go @@ -19,8 +19,10 @@ func (r *Rendering) PlantUML() (string, error) { // direction (PlantUML draws top to bottom or left to right, so a reversed // direction takes its nearest and is noted as not represented; the empty // direction leaves PlantUML's default) and filled from the stated palette by -// keyword family, as the DOT form is. Placement is written as comments, -// PlantUML having no absolute positions; for pinned positions use the DOT form. +// keyword family, as the DOT form is. A rendering some Layout positions draws +// the nodes the DOT form draws: the placed ones, and the unplaced ones too under +// UnplacedStrip. Placement itself is written as comments, PlantUML having no +// absolute positions; for pinned positions use the DOT form. func (r *Rendering) PlantUMLWith(options Options) (string, error) { if !r.Kind.SupportsForm(FormPlantUML) { return "", &WrongFormError{Form: FormPlantUML, Kind: r.Kind, View: r.View} @@ -28,6 +30,10 @@ func (r *Rendering) PlantUMLWith(options Options) (string, error) { if err := options.Palette.check(); err != nil { return "", err } + if err := options.Unplaced.check(); err != nil { + return "", err + } + r = r.settleUnplaced(options.Unplaced, FormPlantUML) w := &plantumlWriter{borders: r.Kind.paletteBorders(), fills: familyFills{palette: options.Palette, tree: r.Kind == KindTree}, labels: labelsOf(r.Roots)} for _, root := range r.Roots { diff --git a/packaging/man/man1/sysml.1 b/packaging/man/man1/sysml.1 index d49d0cb2f2..bc4535b05e 100644 --- a/packaging/man/man1/sysml.1 +++ b/packaging/man/man1/sysml.1 @@ -255,9 +255,10 @@ okabe\-ito, tol\-bright, tol\-muted, tol\-light, brewer\-set2, brewer\-dark2, viridis or cividis; default black and white .TP .BR \-render\-unplaced " \fIplacement\fP" -Where the dot form of a view some Layout positions puts the nodes none does: -omit (default) leaves them undrawn, strip draws them in rows below the -drawing; applies to \-render, \-render\-all and document diagrams +Where a graph form of a view some Layout positions puts the nodes none does: +omit (default) leaves them undrawn in every form, strip draws them, in rows +below the dot drawing; applies to \-render, \-render\-all and document +diagrams .SS Rendering documents .TP .BR \-render\-document " \fIname\fP" @@ -784,11 +785,13 @@ neither Graphviz nor PlantUML is needed to write them. Both are drawn in the black\-and\-white style of the SysML v2 Pilot visualizer; \-render\-palette fills their nodes by keyword family from a colourblind\-safe palette (okabe\-ito, tol\-bright, tol\-muted, tol\-light, brewer\-set2, brewer\-dark2, -viridis or cividis), keeping black text legible on every fill. A DOT drawing -of a view whose members carry DiagramLayout positions pins each at its stated -place and leaves a member with no position undrawn, so nothing lands on a -positioned box; \-render\-unplaced strip draws those members instead, in rows -in a strip below the drawing. The same setting shapes the DOT diagrams of +viridis or cividis), keeping black text legible on every fill. A view whose +members carry DiagramLayout positions draws the placed members and the edges +between them in every graph form, and leaves a member with no position +undrawn: the DOT form pins each at its stated place, so nothing lands on a +positioned box, while Mermaid and PlantUML lay the same members out +themselves. \-render\-unplaced strip draws the unplaced members too, in rows +in a strip below a DOT drawing. The same setting shapes the diagrams of \-render\-document and \-render\-documents. .SH RENDERING A DOCUMENT .RS 2 From ab08be0e214d815cf6a56630cc0fe2af9ce5f426 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 24 Sep 2026 21:35:42 +0000 Subject: [PATCH 3/7] fix(doc): render every document of a set and keep colliding file names apart -render-documents stopped at the first document whose queries failed and wrote nothing; it now compiles and evaluates each document on its own, writes the ones that rendered, stands a page carrying the error in for each that did not so links to it resolve, reports every failure with the document's qualified name and exits 3. File names are planned by the collision tagger -render-all uses, moved to internal/translate/filename, so names meeting under case folding, device stems and over-long names get the tagged form and cross-document links point at it. Co-Authored-By: jason.han --- .../document-multi-valued-cells.fixed.md | 1 + .../positioned-view-every-form.fixed.md | 1 + .../render-documents-partial-set.changed.md | 1 + .../render-documents-same-name.fixed.md | 1 + cmd/sysml/main.go | 5 +- cmd/sysml/render.go | 117 ++------- cmd/sysml/render_document.go | 54 +++- cmd/sysml/render_documents_test.go | 239 +++++++++++++++++- cmd/sysml/render_test.go | 44 +--- cmd/sysml/status.go | 6 +- cmd/sysml/usage.go | 6 +- docs/manual/authoring.md | 12 +- docs/manual/outputs.md | 25 +- docs/manual/troubleshooting.md | 12 +- docs/reference/cli.md | 40 ++- docs/reference/sysml-v1-migration.md | 17 +- internal/doc/docir/crossdoc_test.go | 13 +- internal/doc/docir/evaluate.go | 43 +++- internal/doc/docrender/crossdoc_test.go | 9 +- internal/doc/docrender/html.go | 27 +- internal/doc/docrender/markdown.go | 74 ++++-- internal/frontend/repl/docrender.go | 173 +++++++++---- internal/translate/filename/filename.go | 133 ++++++++++ internal/translate/filename/filename_test.go | 112 ++++++++ packaging/man/man1/sysml.1 | 9 +- tests/hygiene/layering_test.go | 1 + 26 files changed, 878 insertions(+), 297 deletions(-) create mode 100644 changes/unreleased/document-multi-valued-cells.fixed.md create mode 100644 changes/unreleased/positioned-view-every-form.fixed.md create mode 100644 changes/unreleased/render-documents-partial-set.changed.md create mode 100644 changes/unreleased/render-documents-same-name.fixed.md create mode 100644 internal/translate/filename/filename.go create mode 100644 internal/translate/filename/filename_test.go diff --git a/changes/unreleased/document-multi-valued-cells.fixed.md b/changes/unreleased/document-multi-valued-cells.fixed.md new file mode 100644 index 0000000000..7defd61b17 --- /dev/null +++ b/changes/unreleased/document-multi-valued-cells.fixed.md @@ -0,0 +1 @@ +- **A table column over a multi-valued feature renders its values instead of failing the document.** A `Column` whose expression reads a feature declared `[0..*]` (a migrated DocGen table over `attribute :>> tCalibNB = (69.0, 98.0);`) stopped the whole document with "produced 2 values, expected one". The query planner now carries the column's declared multiplicity into execution: a cell holds as many values as the feature admits, written in order and `, `-joined in Markdown, HTML and PDF alike, and an optional feature with no value is an empty cell rather than an error. A column declared `[1]` still refuses zero or several values, naming the bound it expected, and a scalar place — a caption, a `Ref`, a comparison operand — still takes one value. diff --git a/changes/unreleased/positioned-view-every-form.fixed.md b/changes/unreleased/positioned-view-every-form.fixed.md new file mode 100644 index 0000000000..ba7b6e6881 --- /dev/null +++ b/changes/unreleased/positioned-view-every-form.fixed.md @@ -0,0 +1 @@ +- **A view with `DiagramLayout::Layout` positions draws the same nodes in every graph form.** Only the DOT writer left the members no position placed undrawn; the Mermaid and PlantUML forms drew the whole exposed tree, so a migrated diagram exposing a package it never drew expanded into thousands of nodes and tripped the Mermaid workload bound. One placement decision now serves `dot`, `mermaid` and `plantuml`, through `-render`, `-render-all`, document figures, the LSP and gRPC alike: the placed members and the edges between them by default, every member under `-render-unplaced strip`, and each form accounts for what it left out in its own comment syntax (`%% not represented: …` in Mermaid, `' not represented: …` in PlantUML). The bound itself is unchanged. diff --git a/changes/unreleased/render-documents-partial-set.changed.md b/changes/unreleased/render-documents-partial-set.changed.md new file mode 100644 index 0000000000..5107a5c835 --- /dev/null +++ b/changes/unreleased/render-documents-partial-set.changed.md @@ -0,0 +1 @@ +- **`-render-documents` writes every document it can and exits `3` when one could not be rendered.** One document whose query failed stopped the run before anything was written. Each document in the set is now compiled and evaluated on its own: the ones that render are written, a page stating **This document could not be rendered.** and the error stands in for each that does not — so links to it from the other pages resolve rather than dangle — and each failure is reported on stderr as `document could not be rendered: `. The new exit status `3` says the run was carried out in part; `0` still means every document was written and `2` that nothing was (no documents, a model that did not analyse). Same for `-doc-form html`. diff --git a/changes/unreleased/render-documents-same-name.fixed.md b/changes/unreleased/render-documents-same-name.fixed.md new file mode 100644 index 0000000000..69e5b4461c --- /dev/null +++ b/changes/unreleased/render-documents-same-name.fixed.md @@ -0,0 +1 @@ +- **`-render-documents` writes documents whose file names differ in letter case alone instead of stopping.** Two documents `Reports::Summary` and `Reports::SUMMARY` stopped the run with "render to file names that differ only by letter case"; each is now written under its name tagged with `~` and a hash, as `-render-all` writes such views, and the same planner escapes a stem Windows reads as a device and cuts a name too long for a path component. Cross-document links, in a set and in a single document rendered on its own, point at the tagged name where one was needed. Documents sharing a short name in different packages were always written apart, by qualified name, and naming one to `-render-document` by the short name alone is refused with every candidate's qualified name. diff --git a/cmd/sysml/main.go b/cmd/sysml/main.go index b75601323a..8f966e495a 100644 --- a/cmd/sysml/main.go +++ b/cmd/sysml/main.go @@ -560,10 +560,11 @@ func runCLI() int { if status := resolveRunBounds(); status != 0 { return status } - if err := runRenderDocuments(args); err != nil { + status, err := runRenderDocuments(args) + if err != nil { return fail(err) } - return exitHolds + return status } if renderAllDir != "" { diff --git a/cmd/sysml/render.go b/cmd/sysml/render.go index c457ba50ff..3756284a08 100644 --- a/cmd/sysml/render.go +++ b/cmd/sysml/render.go @@ -1,22 +1,19 @@ package main import ( - "crypto/sha256" - "encoding/hex" "errors" "fmt" "os" "path/filepath" "slices" "strings" - "unicode" - "unicode/utf8" "github.com/chzyer/readline" "github.com/Open-MBEE/OpenSysML/internal/frontend/repl" "github.com/Open-MBEE/OpenSysML/internal/ir/view" "github.com/Open-MBEE/OpenSysML/internal/translate/export" + "github.com/Open-MBEE/OpenSysML/internal/translate/filename" "github.com/Open-MBEE/OpenSysML/internal/workspace/model" ) @@ -121,71 +118,26 @@ func runRenderAll(files []string) error { // renderFilenames is the file -render-all writes each view it writes to, by view name; // files meeting letter case aside are tagged until no two meet, or refused if two still do. func renderFilenames(views []model.ViewInfo, form view.Form) (map[string]string, error) { - type plan struct { - name string - form view.Form - tagged bool - } - var plans []*plan + forms := make(map[string]view.Form, len(views)) + var names []string for _, info := range views { written := form if written == "" { written = info.Kind.MachineForm() } if info.Supported && info.Kind.SupportsForm(written) { - plans = append(plans, &plan{name: info.Name, form: written}) - } - } - for { - meeting := map[string][]*plan{} - var keys []string - for _, p := range plans { - key := caseFolded(renderFilename(p.name, p.form, p.tagged)) - if _, seen := meeting[key]; !seen { - keys = append(keys, key) - } - meeting[key] = append(meeting[key], p) - } - progressed := false - for _, key := range keys { - group := meeting[key] - if len(group) < 2 { - continue - } - settled := true - for _, p := range group { - if !p.tagged { - p.tagged, settled, progressed = true, false, true - } - } - if settled { - return nil, fmt.Errorf("views %s and %s have the same rendering path %s", - group[0].name, group[1].name, renderFilename(group[0].name, group[0].form, true)) - } + forms[info.Name] = written + names = append(names, info.Name) } - if !progressed { - break - } - } - filenames := make(map[string]string, len(plans)) - for _, p := range plans { - filenames[p.name] = renderFilename(p.name, p.form, p.tagged) } - return filenames, nil -} - -// caseFolded is text under simple Unicode case folding: two texts fold alike -// exactly when strings.EqualFold holds of them. -func caseFolded(text string) string { - var b strings.Builder - for _, r := range text { - least := r - for f := unicode.SimpleFold(r); f != r; f = unicode.SimpleFold(f) { - least = min(least, f) - } - b.WriteRune(least) + filenames, err := filename.Plan(names, func(name string, tagged bool) string { + return renderFilename(name, forms[name], tagged) + }) + var collision *filename.CollisionError + if errors.As(err, &collision) { + return nil, fmt.Errorf("views %s and %s have the same rendering path %s", collision.Names[0], collision.Names[1], collision.File) } - return b.String() + return filenames, err } // renderOptions is what -render and -render-all write with: the text width, @@ -279,54 +231,17 @@ func renderFilename(name string, form view.Form, tagged bool) string { b.WriteByte(c) } } - filename := b.String() - if stem, _, _ := strings.Cut(filename, "."); windowsDeviceNames[strings.ToUpper(strings.TrimRight(stem, " "))] { - filename = fmt.Sprintf("%%%02X", filename[0]) + filename[1:] - } - ext := renderExtension(form) - if tagged || len(filename)+len(ext) > maxFilenameBytes { - sum := sha256.Sum256([]byte(filename)) - tag := "~" + hex.EncodeToString(sum[:filenameTagBytes]) - filename = cutFilename(filename, maxFilenameBytes-len(ext)-len(tag)) + tag - } - return filename + ext -} - -// maxFilenameBytes is the longest name every common filesystem takes for one path component; -// filenameTagBytes of the encoded name's hash keep a cut name apart from its neighbours. -const ( - maxFilenameBytes = 255 - filenameTagBytes = 8 -) - -// cutFilename is the longest prefix of an encoded filename within n bytes that -// splits neither a UTF-8 sequence nor a `%XX` escape. -func cutFilename(filename string, n int) string { - n = min(n, len(filename)) - for n > 0 && n < len(filename) && !utf8.RuneStart(filename[n]) { - n-- - } - if i := strings.LastIndexByte(filename[:n], '%'); i >= 0 && i > n-3 { - n = i + encoded := b.String() + if filename.DeviceStem(encoded) { + encoded = fmt.Sprintf("%%%02X", encoded[0]) + encoded[1:] } - return filename[:n] + return filename.Fit(encoded, renderExtension(form), tagged) } // unsafeFilenameBytes are the printable bytes a rendering filename encodes: path separators, // the drive colon, the encoding's own `%`, the `.` standing for `::`, and what Windows reserves. const unsafeFilenameBytes = "/\\:%.<>\"|?*" -// windowsDeviceNames are the stems Windows reads as devices whatever the extension, -// trailing spaces and letter case aside: the serial and printer ports include the -// superscript digits Windows counts among them. -var windowsDeviceNames = map[string]bool{ - "CON": true, "PRN": true, "AUX": true, "NUL": true, - "COM0": true, "COM1": true, "COM2": true, "COM3": true, "COM4": true, "COM5": true, "COM6": true, "COM7": true, "COM8": true, "COM9": true, - "COM¹": true, "COM²": true, "COM³": true, - "LPT0": true, "LPT1": true, "LPT2": true, "LPT3": true, "LPT4": true, "LPT5": true, "LPT6": true, "LPT7": true, "LPT8": true, "LPT9": true, - "LPT¹": true, "LPT²": true, "LPT³": true, -} - func renderExtension(form view.Form) string { switch form { case view.FormMermaid: diff --git a/cmd/sysml/render_document.go b/cmd/sysml/render_document.go index 6e01856c19..88c06f4b3b 100644 --- a/cmd/sysml/render_document.go +++ b/cmd/sysml/render_document.go @@ -89,17 +89,44 @@ func pdfOptions() (docpdf.Options, error) { // runRenderDocuments renders every document definition of the model named on // the command line as linked files in the directory -render-documents names, // so cross-document references resolve on disk. -func runRenderDocuments(files []string) error { +// runRenderDocuments renders every document of the model named on the command +// line into -render-documents as a linked set, writing the pages of the +// documents that render and, for each that does not, a page stating why; it +// returns the status of the run and, when nothing was written, what stopped it. +func runRenderDocuments(files []string) (int, error) { + documents, form, err := renderDocumentSet(files) + if err != nil { + return exitUnevaluable, err + } + if err := os.MkdirAll(renderDocsDir, 0o750); err != nil { + return exitUnevaluable, fmt.Errorf("create rendering directory %s: %w", renderDocsDir, err) + } + if err := commitDocumentSet(documents, form); err != nil { + return exitUnevaluable, err + } + status := exitHolds + for _, document := range documents { + if document.Err != nil { + fmt.Fprintf(os.Stderr, "%sdocument %s could not be rendered: %v\n", commandPrefix, source.QualifiedNameText(document.Name), document.Err) + status = exitPartial + } + } + return status, nil +} + +// renderDocumentSet renders the model's documents in the form -doc-form names, +// the stylesheets of an HTML set among them, without writing anything. +func renderDocumentSet(files []string) ([]repl.RenderedDocument, string, error) { form, err := documentSetForm() if err != nil { - return err + return nil, "", err } if len(files) == 0 { - return errors.New("no model to render; name the files the documents are declared in, as `sysml model.sysml -render-documents rendered`") + return nil, "", errors.New("no model to render; name the files the documents are declared in, as `sysml model.sysml -render-documents rendered`") } sess, err := loadRenderingModel(files) if err != nil { - return err + return nil, "", err } // A set links its stylesheets as files beside the pages, so a reader // downloads each once and edits it in one place. @@ -108,7 +135,7 @@ func runRenderDocuments(files []string) error { if form == docFormHTML { links, assets, err := setStylesheets() if err != nil { - return err + return nil, "", err } sheets = assets opts := documentOptions() @@ -117,22 +144,18 @@ func runRenderDocuments(files []string) error { opts.NoDefaultStylesheet = true documents, err = sess.RenderDocumentSetHTML(opts) if err != nil { - return err + return nil, "", err } } else { documents, err = sess.RenderDocumentSetMarkdown(markdownOptions()) if err != nil { - return err + return nil, "", err } } if len(documents) == 0 { - return errors.New("the model declares no documents; nothing was rendered") - } - documents = append(documents, sheets...) - if err := os.MkdirAll(renderDocsDir, 0o750); err != nil { - return fmt.Errorf("create rendering directory %s: %w", renderDocsDir, err) + return nil, "", errors.New("the model declares no documents; nothing was rendered") } - return commitDocumentSet(documents, form) + return append(documents, sheets...), form, nil } // documentOptions carries the flags shaping the document itself, leaving its @@ -468,8 +491,11 @@ func commitDocumentSet(documents []repl.RenderedDocument, form string) error { } path := filepath.Join(renderDocsDir, document.FileName) what := "" + if document.Err != nil { + what = ", a page stating why the document could not be rendered" + } if replaced[i] { - what = ", replaced the existing file" + what += ", replaced the existing file" } fmt.Fprintf(os.Stderr, "wrote %s (%s, %d bytes%s)\n", path, setForm(document, form), len(documentBytes(document)), what) } diff --git a/cmd/sysml/render_documents_test.go b/cmd/sysml/render_documents_test.go index 40832ea026..5afd5d122b 100644 --- a/cmd/sysml/render_documents_test.go +++ b/cmd/sysml/render_documents_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/Open-MBEE/OpenSysML/internal/syntax/source" + "github.com/Open-MBEE/OpenSysML/internal/translate/filename" ) // linkedModel declares two documents referencing each other's content, so the @@ -453,17 +454,24 @@ func TestReplaceFileReplacesExistingTarget(t *testing.T) { } } -// TestRenderDocumentsCaseCollidingNames checks documents whose file names -// differ only by letter case are rejected before anything is written, so a -// set renders the same on case-sensitive and case-insensitive filesystems. +// TestRenderDocumentsCaseCollidingNames checks two documents whose names meet +// letter case aside are each written under a tagged name, so a set survives a +// filesystem that folds case, and the links between them point at the tags. func TestRenderDocumentsCaseCollidingNames(t *testing.T) { binary := buildCLI(t) model := `package Reports { private import DocumentQueries::*; private import ScalarValues::*; + ref shouting : WEEKLY; + part def Weekly :> Document { attribute redefines title = "Weekly"; + part intro : Paragraph { + part see : Ref { + ref redefines target = shouting; + } + } } part def WEEKLY :> Document { attribute redefines title = "WEEKLY"; @@ -472,11 +480,228 @@ func TestRenderDocumentsCaseCollidingNames(t *testing.T) { ` dir := filepath.Join(t.TempDir(), "rendered") got := check(t, binary, model, "-render-documents", dir) - if got.status != 2 || !strings.Contains(got.stderr, "differ only by letter case") { - t.Fatalf("exit = %d stderr = %q", got.status, got.stderr) + wantReport(t, got, 0, "Reports-Weekly~", "Reports-WEEKLY~") + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + var weekly, shouting string + for _, entry := range entries { + switch { + case strings.HasPrefix(entry.Name(), "Reports-Weekly~"): + weekly = entry.Name() + case strings.HasPrefix(entry.Name(), "Reports-WEEKLY~"): + shouting = entry.Name() + default: + t.Errorf("unexpected file %s", entry.Name()) + } + } + if weekly == "" || shouting == "" || filename.CaseFolded(weekly) == filename.CaseFolded(shouting) { + t.Fatalf("files = %q and %q, want two names distinct letter case aside", weekly, shouting) + } + page, err := os.ReadFile(filepath.Join(dir, weekly)) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(page), "]("+shouting+")") { + t.Errorf("the link does not point at the tagged file %s:\n%s", shouting, page) + } +} + +// TestRenderDocumentsSameShortName checks documents of one short name in +// different packages each get their own file, and a link to one of them +// lands on that one. +func TestRenderDocumentsSameShortName(t *testing.T) { + binary := buildCLI(t) + model := `package Reports { + private import DocumentQueries::*; + private import ScalarValues::*; + + package Alpha { + part def Summary :> Document { + attribute redefines title = "Alpha Summary"; + } + } + package Beta { + part def Summary :> Document { + attribute redefines title = "Beta Summary"; + } + } + ref beta : Beta::Summary; + + part def Index :> Document { + attribute redefines title = "Index"; + part intro : Paragraph { + part see : Ref { + ref redefines target = beta; + } + } + } +} +` + dir := filepath.Join(t.TempDir(), "rendered") + got := check(t, binary, model, "-render-documents", dir) + wantReport(t, got, 0, "Reports-Alpha-Summary.md", "Reports-Beta-Summary.md", "Reports-Index.md") + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + if len(entries) != 3 { + t.Errorf("wrote %d files, want 3", len(entries)) + } + for file, title := range map[string]string{ + "Reports-Alpha-Summary.md": "# Alpha Summary", + "Reports-Beta-Summary.md": "# Beta Summary", + } { + page, err := os.ReadFile(filepath.Join(dir, file)) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(page), title) { + t.Errorf("%s lacks %q:\n%s", file, title, page) + } + } + index, err := os.ReadFile(filepath.Join(dir, "Reports-Index.md")) + if err != nil { + t.Fatal(err) } - if _, err := os.Stat(dir); !os.IsNotExist(err) { - t.Errorf("a rejected set created the directory: %v", err) + if !strings.Contains(string(index), "](Reports-Beta-Summary.md)") { + t.Errorf("the link does not name the Beta document's file:\n%s", index) + } +} + +// partialModel declares three documents, one of which fails to evaluate: its +// table reads a [1] attribute the row leaves unbound. The others link to it. +const partialModel = `package Reports { + private import DocumentQueries::*; + private import KerML::Root::Element; + private import ScalarValues::*; + + part def Scenario { + attribute duration : Real; + } + part campaign { + part idle : Scenario; + } + + calc def Timings :> Query { + in root : Element = campaign; + Project( + source = WhereType(source = Descendants(source = root, maxDepth = 1), type = "PartUsage"), + properties = ("name"), + columns = (Column(name = "duration", expression = Scenario::duration)) + ) + } + + ref brokenDoc : Broken; + + part def Broken :> Document { + attribute redefines title = "Broken Timings"; + part timings : Table { + calc rows : Timings; + } + } + + part def First :> Document { + attribute redefines title = "First"; + part intro : Paragraph { + part see : Ref { + ref redefines target = brokenDoc; + } + } + } + + part def Second :> Document { + attribute redefines title = "Second"; + part intro : Paragraph { + attribute redefines text = "second"; + } + } +} +` + +// TestRenderDocumentsPartialSet checks a set with one document that cannot be +// rendered still writes the others, writes a page stating the error where the +// failed document's links land, names the failure on stderr, and exits 3. +func TestRenderDocumentsPartialSet(t *testing.T) { + binary := buildCLI(t) + for _, form := range []struct { + name, ext string + args []string + link string + }{ + {"markdown", ".md", nil, "](Reports-Broken.md)"}, + {"html", ".html", []string{"-doc-form", "html"}, `href="Reports-Broken.html"`}, + } { + t.Run(form.name, func(t *testing.T) { + dir := filepath.Join(t.TempDir(), "rendered") + got := check(t, binary, partialModel, append([]string{"-render-documents", dir}, form.args...)...) + wantReport(t, got, 3, + "Reports-First"+form.ext+" (", + "Reports-Second"+form.ext+" (", + "Reports-Broken"+form.ext+" (", + "a page stating why the document could not be rendered", + "sysml: document Reports::Broken could not be rendered: ", + "column duration", "duration") + if strings.Count(got.stderr, "could not be rendered:") != 1 { + t.Errorf("want the one failure named once:\n%s", got.stderr) + } + second, err := os.ReadFile(filepath.Join(dir, "Reports-Second"+form.ext)) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(second), "second") { + t.Errorf("the rendered document lacks its text:\n%s", second) + } + first, err := os.ReadFile(filepath.Join(dir, "Reports-First"+form.ext)) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(first), form.link) { + t.Errorf("the link to the failed document is not %s:\n%s", form.link, first) + } + broken, err := os.ReadFile(filepath.Join(dir, "Reports-Broken"+form.ext)) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{"Broken Timings", "This document could not be rendered.", "duration"} { + if !strings.Contains(string(broken), want) { + t.Errorf("the failed document's page lacks %q:\n%s", want, broken) + } + } + }) + } +} + +// TestRenderDocumentAmbiguousName checks a short name held by documents in +// several packages is refused with every candidate's qualified name. +func TestRenderDocumentAmbiguousName(t *testing.T) { + binary := buildCLI(t) + model := `package Reports { + private import DocumentQueries::*; + private import ScalarValues::*; + + package Alpha { + part def Summary :> Document { + attribute redefines title = "Alpha Summary"; + } + } + package Beta { + part def Summary :> Document { + attribute redefines title = "Beta Summary"; + } + } +} +` + got := check(t, binary, model, "-render-document", "Summary") + wantReport(t, got, 2, `symbol "Summary" is ambiguous: Reports::Alpha::Summary, Reports::Beta::Summary`, "use a qualified name") + if got.stdout != "" { + t.Errorf("stdout = %q, want nothing", got.stdout) + } + got = check(t, binary, model, "-render-document", "Reports::Beta::Summary") + wantReport(t, got, 0) + if !strings.Contains(got.stdout, "# Beta Summary") { + t.Errorf("the qualified name did not render the Beta document:\n%s", got.stdout) } } diff --git a/cmd/sysml/render_test.go b/cmd/sysml/render_test.go index 0259d74d9e..2899541419 100644 --- a/cmd/sysml/render_test.go +++ b/cmd/sysml/render_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/Open-MBEE/OpenSysML/internal/ir/view" + "github.com/Open-MBEE/OpenSysML/internal/translate/filename" "github.com/Open-MBEE/OpenSysML/internal/workspace/model" ) @@ -674,11 +675,11 @@ func TestRenderFilenameEncodesWhatAFilesystemRefuses(t *testing.T) { func TestRenderFilenameFitsAPathComponent(t *testing.T) { long := "TMT::" + strings.Repeat("'Acquire Telescope Pointing w/NSEN'::", 8) got := renderFilename(long+"first", view.FormDot, false) - if len(got) != maxFilenameBytes || !strings.HasSuffix(got, ".dot") { - t.Errorf("renderFilename(long) = %q (%d bytes), want %d ending in .dot", got, len(got), maxFilenameBytes) + if len(got) != filename.Max || !strings.HasSuffix(got, ".dot") { + t.Errorf("renderFilename(long) = %q (%d bytes), want %d ending in .dot", got, len(got), filename.Max) } - if i := strings.LastIndexByte(got, '~'); i < 0 || len(got)-i != 1+2*filenameTagBytes+len(".dot") { - t.Errorf("renderFilename(long) = %q lacks a %d-byte hash tag before the extension", got, 2*filenameTagBytes) + if i := strings.LastIndexByte(got, '~'); i < 0 || len(got)-i != 1+2*filename.TagBytes+len(".dot") { + t.Errorf("renderFilename(long) = %q lacks a %d-byte hash tag before the extension", got, 2*filename.TagBytes) } if got != renderFilename(long+"first", view.FormDot, false) { t.Errorf("renderFilename(long) is not deterministic") @@ -696,7 +697,7 @@ func TestRenderFilenameFitsAPathComponent(t *testing.T) { {a + "aaö" + a, a + "aaö"}, } { got := renderFilename(tc.name, view.FormDot, false) - if len(got) > maxFilenameBytes || !strings.HasPrefix(got, tc.stem+"~") { + if len(got) > filename.Max || !strings.HasPrefix(got, tc.stem+"~") { t.Errorf("renderFilename(%q) = %q (%d bytes), want the stem %q", tc.name, got, len(got), tc.stem) } } @@ -758,11 +759,11 @@ func TestRenderFilenamesMeetOnlyOnce(t *testing.T) { t.Errorf("view %s named like the tag is written to %q, want a tagged name other than %s", tagName, got[tagName], tagged) } folded := map[string]string{} - for name, filename := range got { - if other, met := folded[caseFolded(filename)]; met { - t.Errorf("views %s and %s are both written to %s", other, name, filename) + for name, file := range got { + if other, met := folded[filename.CaseFolded(file)]; met { + t.Errorf("views %s and %s are both written to %s", other, name, file) } - folded[caseFolded(filename)] = name + folded[filename.CaseFolded(file)] = name } sequence := model.ViewInfo{Name: "Demo::report", Kind: view.KindSequence, Supported: true} @@ -824,36 +825,13 @@ func TestRenderAllTagsPathsMeetingUnderCaseFolding(t *testing.T) { for _, name := range []string{tc.first, tc.second} { stem := "Demo." + strings.Trim(name, "'") + "~" i := slices.IndexFunc(names, func(f string) bool { return strings.HasPrefix(f, stem) && strings.HasSuffix(f, ".dot") }) - if i < 0 || len(names[i]) != len(stem)+2*filenameTagBytes+len(".dot") { + if i < 0 || len(names[i]) != len(stem)+2*filename.TagBytes+len(".dot") { t.Errorf("files = %v, want one hash-tagged %s*.dot", names, stem) } } } } -// Case folding is Unicode's simple folding, which strings.EqualFold decides: -// a final sigma folds with a sigma, a Kelvin sign with a k, and a name that -// differs in more than case folds apart. -func TestCaseFoldedAgreesWithEqualFold(t *testing.T) { - for _, tc := range []struct { - a, b string - want bool - }{ - {"Report.dot", "report.dot", true}, - {"σ", "ς", true}, - {"Σ", "ς", true}, - {"k", "\u212a", true}, - {"ß", "ẞ", true}, - {"ſ", "S", true}, - {"i", "İ", false}, - {"Report.dot", "Reports.dot", false}, - } { - if got := caseFolded(tc.a) == caseFolded(tc.b); got != tc.want || got != strings.EqualFold(tc.a, tc.b) { - t.Errorf("caseFolded(%q) == caseFolded(%q) is %v, want %v, as EqualFold says %v", tc.a, tc.b, got, tc.want, strings.EqualFold(tc.a, tc.b)) - } - } -} - // The OOSEM view definitions carry their filter and rendering; a usage only // exposes, and the inherited filter keeps everything else out of the artifact. func TestRenderAllOOSEMViews(t *testing.T) { diff --git a/cmd/sysml/status.go b/cmd/sysml/status.go index da639ed7f2..7801d95ca1 100644 --- a/cmd/sysml/status.go +++ b/cmd/sysml/status.go @@ -7,12 +7,14 @@ import ( "github.com/chzyer/readline" ) -// Exit statuses of any run: a verdict the model decided false is 1, and anything -// that stopped the run from being carried out at all is 2. +// Exit statuses of any run: a verdict the model decided false is 1, anything +// that stopped the run from being carried out at all is 2, and a run carried +// out in part, writing what it could and naming what it could not, is 3. const ( exitHolds = 0 exitFailed = 1 exitUnevaluable = 2 + exitPartial = 3 ) // fail reports on stderr what stopped the run, returning the status of a run diff --git a/cmd/sysml/usage.go b/cmd/sysml/usage.go index 5d22bc4572..52df084ee4 100644 --- a/cmd/sysml/usage.go +++ b/cmd/sysml/usage.go @@ -482,6 +482,10 @@ func doc() usage.Doc { usage.Entry("2", "What was asked could not be carried out at all — an unreadable "+ "file, a model that did not analyse cleanly, an unresolved name, a "+ "failed conversion."), + usage.Entry("3", "Part of what was asked was carried out: a -render-documents set "+ + "in which some document could not be rendered. The others were "+ + "written, a page stating the error stands in for each that was not, "+ + "and each failure is reported with the document's qualified name."), }, }, { Title: "Output streams", @@ -599,7 +603,7 @@ func registerFlags(fs *flag.FlagSet) { fs.StringVar(&renderUnplaced, "render-unplaced", "", "Where a graph form of a view some Layout positions puts the nodes none does: omit (default) leaves them undrawn in every form, strip draws them, in rows below the dot drawing; applies to -render, -render-all and document diagrams") fs.StringVar(&renderDoc, "render-document", "", "Compile this document definition, run its queries and write the rendered document") - fs.StringVar(&renderDocsDir, "render-documents", "", "Render every document definition, linked to one another, into this directory") + fs.StringVar(&renderDocsDir, "render-documents", "", "Render every document definition, linked to one another, into this directory; a document that cannot be rendered gets a page stating why and the run exits 3") fs.StringVar(&docForm, "doc-form", "", "Form the documents are written in: markdown (default), html or pdf, which drives an external converter") fs.StringVar(&diagramForm, "diagram-form", "", "Form the documents' graph-shaped diagrams are written in: mermaid (default), dot or plantuml; a table-kind view is a table either way") fs.BoolVar(&pdfTitlePage, "doc-title-page", false, "Put the document title on a page of its own (html or pdf)") diff --git a/docs/manual/authoring.md b/docs/manual/authoring.md index a304bf80b9..2cf925590b 100644 --- a/docs/manual/authoring.md +++ b/docs/manual/authoring.md @@ -142,11 +142,13 @@ with the block's stable anchor — a destination like a root target links to the file alone. The file name is deterministic: the target document's fully qualified name with `::` replaced by `-` and any byte outside ASCII letters, digits and `_` escaped as `.XX` (uppercase hex), -plus `.md`. Render the whole set with `-render-documents ` so the links -resolve on disk. Rendering a single document that references another still -succeeds — the link points at the expected file name of the unrendered -target, and it dangles until that document is rendered into the same -directory. An unknown target is a typed planning error, and a target usage +plus `.md`; where two names would meet in one file (see +[Multi-document sets](outputs.md#multi-document-sets)) the link carries the +tagged name the set writes. Render the whole set with `-render-documents +` so the links resolve on disk. Rendering a single document that +references another still succeeds — the link points at the file name the set +gives the unrendered target, and it dangles until that document is rendered +into the same directory. An unknown target is a typed planning error, and a target usage typed by more than one document definition is an ambiguous-target error; both carry the reference's source location. diff --git a/docs/manual/outputs.md b/docs/manual/outputs.md index 35f90e8c30..a16c449ca3 100644 --- a/docs/manual/outputs.md +++ b/docs/manual/outputs.md @@ -48,12 +48,25 @@ $ sysml reports.sysml -render-documents rendered File names are deterministic: the document's fully qualified name with `::` replaced by `-`, any byte outside ASCII letters, digits and `_` escaped as -`.XX` (uppercase hex), plus `.md`. Cross-document references (see -[the authoring chapter](authoring.md)) therefore resolve as relative links -between the written files, and repeated runs write identical bytes. Rendering -a cross-referencing document on its own still succeeds; its external links -point at the targets' expected file names and dangle until those documents -are rendered into the same directory. +`.XX` (uppercase hex), plus `.md`, so documents sharing a short name in +different packages get distinct files. Two names whose files would meet on a +case-insensitive file system, a stem that would name a Windows device, and a +name too long for a file system are written under a tagged name carrying a +`~` and a hash of the whole name, as `-render-all` writes its views. +Cross-document references (see [the authoring chapter](authoring.md)) +therefore resolve as relative links between the written files, tagged names +included, and repeated runs write identical bytes. Rendering a +cross-referencing document on its own still succeeds; its external links +point at the file names the set gives the targets and dangle until those +documents are rendered into the same directory. + +Each document in the set is compiled and evaluated on its own. One that +cannot be rendered — a query column with no value for a row, a diagram past +its form's limit — does not stop the others: they are written, a page carrying +the document's title, **This document could not be rendered.** and the error +is written in its place so links to it resolve, each failure is reported on +stderr as `document could not be rendered: `, and the +run exits with status 3 rather than 0. ## HTML diff --git a/docs/manual/troubleshooting.md b/docs/manual/troubleshooting.md index 607b2abb24..aae61b0129 100644 --- a/docs/manual/troubleshooting.md +++ b/docs/manual/troubleshooting.md @@ -2,12 +2,16 @@ ## How errors surface -Document generation fails loudly and early, never partially. Every mistake is +Document generation fails loudly, never silently. Every mistake is a **typed error**. Planning problems (structure, query references, bindings) surface when the document is compiled, as `document-plan-*` diagnostics in the editor and as source-located errors from the CLI. Execution problems (a query that cannot run) stop the render with the query error's -message. A document that cannot be rendered exits `2`, and nothing is written. +message. A single document that cannot be rendered exits `2`, and nothing is +written. In a `-render-documents` set each document stands on its own: the +ones that render are written, a page stating the error stands in for each +that does not, every failure is listed on stderr with the document's qualified +name, and the run exits `3`. ```console $ sysml report.sysml -render-document E::R @@ -118,8 +122,8 @@ position; they are tracked in the project's compliance record. a content block or the root of another document (see the authoring chapter's cross-document pattern), and `-render-documents` writes the linked set together. Rendering one document alone still succeeds, but its - cross-document links point at the target's expected file name and dangle - until that document is rendered into the same directory. + cross-document links point at the file name the set gives the target and + dangle until that document is rendered into the same directory. - **Captions are emphasis in Markdown, elements in HTML and PDF.** The Markdown dialect writes a caption as an emphasized paragraph ahead of its table, diagram or formula, with no marker distinguishing it from an diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 2398a270a0..b46303490d 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -225,7 +225,7 @@ reported, so a script that reads it takes the output from the first `{`. | `--render-document ` | | Compile a document definition (a `part def` specializing `DocumentQueries::Document`), run its queries against the model, render its diagram blocks through the view engine and write the result as CommonMark Markdown, as `%render-document` does. Paragraphs may hold inline runs (`Span` with a `plain`/`emphasis`/`strong`/`code` style, `Link` to a URL, `Ref` linking to another content block's anchor); a query-backed paragraph or list styles its projected values through nested `SpanColumn`/`LinkColumn` column runs; a table with a `groupBy` column writes one subtable per group value, with the query's projected properties and computed `Column` names as its columns. A `Diagram` block embeds a declared view, or an element with a stated rendering kind, as a fenced ` ```mermaid ` block (a fenced ` ```dot ` block of Graphviz DOT under `-diagram-form dot`, a ` ```plantuml ` block under `-diagram-form plantuml`; a table-kind view as a pipe table whichever form), with an optional caption and `TB`/`LR`/`RL`/`BT` flow direction. Markdown is the default form; `-doc-form html` renders the same document tree as semantic HTML (see [Rendering a document as HTML](#rendering-a-document-as-html)) and `-doc-form pdf` converts the Markdown (see [Rendering a document as PDF](#rendering-a-document-as-pdf)). Combined with `--instantiate`, the document's queries run over the objects created (see [Rendering a document over objects](#rendering-a-document-over-objects)). `-json` does not apply. See the [document generation manual](../manual/README.md) | | `--doc-form ` | | Form `--render-document` writes: `markdown` (default), `html`, rendered from the document tree itself (see [Rendering a document as HTML](#rendering-a-document-as-html)), or `pdf`, which drives an external converter | | `--diagram-form ` | | Form the graph-shaped diagram blocks of `--render-document` and `--render-documents` are written in: `mermaid` (default), `dot`, Graphviz DOT for a toolchain that lays diagrams out with Graphviz, produced without Graphviz installed, or `plantuml`, PlantUML in the Pilot visualizer's B&W style, produced without a PlantUML jar. Applies to every diagram of the document in every `--doc-form`; a table-kind view is a table whichever form, and a `sequence` diagram, which has no DOT form, is refused under `dot` | -| `--render-documents ` | | Render every document definition the model declares as a linked set into the directory, one file per document, so cross-document references resolve on disk. `--doc-form html` writes the set as HTML pages linking shared stylesheet files written beside them | +| `--render-documents ` | | Render every document definition the model declares as a linked set into the directory, one file per document, so cross-document references resolve on disk; a document that cannot be rendered gets a page stating why and the run exits 3. `--doc-form html` writes the set as HTML pages linking shared stylesheet files written beside them | | `--doc-title-page` | | Put the document title on a page of its own (`--doc-form html` or `pdf`) | | `--doc-toc` | | Write a table of contents ahead of the content (`--doc-form html` or `pdf`) | | `--doc-number-sections` | | Number the section headings hierarchically (`--doc-form html` or `pdf`) | @@ -680,22 +680,43 @@ sysml model.sysml -render Views::vehicleView -render-form text `-render-documents ` renders every document definition the loaded model declares into the directory, one Markdown file per document, in fully-qualified-name order. Each file name is the document's fully qualified name with `::` replaced by `-`, any byte outside ASCII letters, -digits and `_` escaped as `.XX` (uppercase hex), plus `.md`. The names are deterministic, so -cross-document references (see [the authoring chapter](../manual/authoring.md)) resolve as relative -links between the written files, and repeated runs write identical bytes. +digits and `_` escaped as `.XX` (uppercase hex), plus `.md`; so two documents that share a short +name in different packages (`Reports::Alpha::Summary` and `Reports::Beta::Summary`) get two +files. A stem that would name a Windows device (`CON`, `AUX`) is escaped, and a name too long for +a file system is cut to a fixed prefix plus `~` and a hash of the whole name. Two documents +whose files would meet on a case-insensitive file system (`Reports::Summary` and +`Reports::SUMMARY`) are both written under that tagged `~hash` form, as `-render-all` does, so +the set never puts two documents in one file. The names are deterministic, so cross-document +references (see [the authoring chapter](../manual/authoring.md)) resolve as relative links between +the written files — to the tagged name where one was needed — and repeated runs write identical +bytes. ```bash sysml model.sysml -render-documents rendered ``` -The directory is created if needed; written paths go to stderr and stdout stays empty. A -model that declares no documents, declares two documents with the same name, or does not analyse -cleanly stops the run with status 2. `-render-documents` cannot be combined with +The directory is created if needed; written paths go to stderr and stdout stays empty. A model +that declares no documents or does not analyse cleanly stops the run with status 2 before anything +is written. Once the set is being rendered, each document is compiled and evaluated on its own: +one that fails — a query whose column has no value for a row, a diagram past a form's limit — does +not stop the others. The run writes every document that rendered, writes in place of each one +that did not a page carrying its title, **This document could not be rendered.** and the error, so +links to it from other pages resolve to that explanation rather than dangling, and then reports +each failure on stderr as `document could not be rendered: ` and exits +with status 3 (see [Exit status](#exit-status)). `-render-documents` cannot be combined with `-render-document`, `-render`, `-render-all`, `-o`, `-convert`, a query flag, or a check flag other than `-instantiate` (see [Rendering a document over objects](#rendering-a-document-over-objects)). Rendering a single document with `-render-document` still succeeds when it has cross-document -references: the links point at the targets' expected file names and dangle until those documents -are rendered into the same directory. +references: the links point at the file names the set would give the targets and dangle until +those documents are rendered into the same directory. + +A document named by its short name alone when the model declares more than one of that name is +ambiguous, and the error names every candidate so one can be copied: + +```bash +$ sysml model.sysml -render-document Summary +sysml: symbol "Summary" is ambiguous: Reports::Alpha::Summary, Reports::Beta::Summary (use a qualified name) +``` `-render-document` takes as many model files as the document needs, loaded as one model, so a document can query elements declared in sibling files: @@ -2022,6 +2043,7 @@ the one place it is written down; [the guide](../guide/) links here. | `0` | What was asked for was done: every file loaded and analysed cleanly, every `-e` expression produced a value (``, the model-level value of an expression over a feature the model leaves open, is one), every check held, a conversion was written. Warnings leave the status `0`. | | `1` | The model answered false: a constraint, requirement or satisfaction assertion the model decided did not hold. Only a verdict reports this status. | | `2` | What was asked for could not be done, so the model answered nothing: a file that could not be read, a model that did not analyse cleanly, an object whose feature values did not materialize, an unresolved name, a check that could not be made (including a condition that is ``: it is reported as `no value`, naming the feature the model leaves open, never as a verdict), an exploration that hit its budget before every linearization was tried, a conversion that could not be written because the RDF graph cannot rebuild a source construct, a misused flag or an invalid `OPENSYSML_MAX_*` value. | +| `3` | Part of what was asked for was done: a `-render-documents` set in which at least one document could not be rendered. Every document that rendered was written, a page stating the error was written in place of each one that did not, and each failure was reported on stderr with the document's qualified name. Only a set of independent outputs reports this status. | ```bash $ printf '%s\n' 'constraint MassBudget { 1 > 2 }' > model.sysml diff --git a/docs/reference/sysml-v1-migration.md b/docs/reference/sysml-v1-migration.md index 1d393aabd4..a82e46991c 100644 --- a/docs/reference/sysml-v1-migration.md +++ b/docs/reference/sysml-v1-migration.md @@ -647,9 +647,20 @@ drawn from its graph, each positioned where the MTIP layout put it when `-layout draw stays out of the figure rather than expanding into its whole contents, and `-render-unplaced strip` adds the unplaced elements; a diagram the migration left out of the document (empty, or rendered as textual notation) is absent from the -render and the report says why, so a rendered document holds no empty figure. Styling beyond -what the model carries — a cover image, a tool's fonts, its header and footer — is not -invented; `-html-theme` and `-html-css` take a stylesheet of your own. +render and the report says why, so a rendered document holds no empty figure. A table cell over +a multi-valued slot (`attribute :>> tCalibNB = (69.0, 98.0);` under a column declared `[0..*]`) +lists its values in order, `69, 98`, as DocGen's did, and one over a slot with no value is empty. +Styling beyond what the model carries — a cover image, a tool's fonts, its header and footer — +is not invented; `-html-theme` and `-html-css` take a stylesheet of your own. + +The whole set of a model's documents is rendered with `-render-documents ` (see +[the CLI reference](cli.md#rendering-a-view)): every document in one directory, linked to one +another. A model whose documents repeat a short name across packages — DocGen templates +instantiated in several places — renders each to its own file, since the file is named by the +qualified name; naming one of them to `-render-document` by the short name alone is refused +with the qualified name of every candidate. A document that cannot be rendered does not stop the +set: the others are written, a page stating the error stands in for it, and the run lists each +such document and exits `3`. The mapping has been run over the XMI of the [OpenMBEE TMT SysML model](https://github.com/Open-MBEE/TMT-SysML-Model) (27 MB; 44,600 elements once the nodes and edges of its behaviors are counted): it writes 7 MB diff --git a/internal/doc/docir/crossdoc_test.go b/internal/doc/docir/crossdoc_test.go index 6289c85f14..c3d47a86a1 100644 --- a/internal/doc/docir/crossdoc_test.go +++ b/internal/doc/docir/crossdoc_test.go @@ -67,14 +67,19 @@ func TestEvaluateCrossDocumentRefs(t *testing.T) { func TestEvaluateSetEmitsCrossDocumentAnchors(t *testing.T) { fixture := loadEvaluationFixture(t, crossDocumentFixture) plans := []*docplan.Plan{fixture.plan(t, "Appendix"), fixture.plan(t, "Report")} - documents, err := EvaluateSet(plans, fixture.context(), queryexec.Options{}, nil) + outcomes, err := EvaluateSet(plans, fixture.context(), queryexec.Options{}, nil) if err != nil { t.Fatalf("evaluate set: %v", err) } - if len(documents) != 2 { - t.Fatalf("documents = %d, want 2", len(documents)) + if len(outcomes) != 2 { + t.Fatalf("documents = %d, want 2", len(outcomes)) } - appendix := documents[0] + for _, outcome := range outcomes { + if outcome.Err != nil { + t.Fatalf("evaluate %s: %v", outcome.Name, outcome.Err) + } + } + appendix := outcomes[0].Document if appendix.Name() != "Observatory::Appendix" { t.Fatalf("first document = %s", appendix.Name()) } diff --git a/internal/doc/docir/evaluate.go b/internal/doc/docir/evaluate.go index 5644988f61..f4d76825b7 100644 --- a/internal/doc/docir/evaluate.go +++ b/internal/doc/docir/evaluate.go @@ -43,15 +43,25 @@ func EvaluateLinked( return evaluate(plan, context, options, text, external[plan.Name()]) } +// Evaluated is the outcome of evaluating one plan of a set: the document, or +// under Err the error that kept the plan named Name from evaluating. +type Evaluated struct { + Name string + Document *Document + Err error +} + // EvaluateSet evaluates a set of compiled document plans together, so a // content block one document references from another carries its anchor in -// the rendered target document. +// the rendered target document. Each plan evaluates on its own: one that +// fails leaves the others' documents whole and is reported in its outcome. +// A plan that is not compiled fails the set. func EvaluateSet( plans []*docplan.Plan, context queryexec.Context, options queryexec.Options, text view.SourceText, -) ([]*Document, error) { +) ([]Evaluated, error) { external := make(map[string]map[string]bool) for _, plan := range plans { if !plan.Compiled() { @@ -60,15 +70,32 @@ func EvaluateSet( collectCrossAnchors(plan.Content(), external) } context = sharingRelationshipTables(context) - documents := make([]*Document, 0, len(plans)) + outcomes := make([]Evaluated, 0, len(plans)) for _, plan := range plans { document, err := evaluate(plan, context, options, text, external[plan.Name()]) - if err != nil { - return nil, err - } - documents = append(documents, document) + outcomes = append(outcomes, Evaluated{Name: plan.Name(), Document: document, Err: err}) + } + return outcomes, nil +} + +// Unrendered is the document a set writes in place of the one named name, +// titled title, that could not be rendered: one paragraph stating why, so a +// link into it lands on the reason rather than on nothing. +func Unrendered(name, title string, err error) *Document { + if title == "" { + title = name + } + return &Document{ + name: name, + title: title, + content: []Content{{ + kind: ContentParagraph, + runs: []TextRun{ + {kind: RunStrong, text: "This document could not be rendered."}, + {kind: RunPlain, text: err.Error()}, + }, + }}, } - return documents, nil } func evaluate( diff --git a/internal/doc/docrender/crossdoc_test.go b/internal/doc/docrender/crossdoc_test.go index c54f468a6c..8449bf6118 100644 --- a/internal/doc/docrender/crossdoc_test.go +++ b/internal/doc/docrender/crossdoc_test.go @@ -79,12 +79,19 @@ func fixtureDocumentSet(t *testing.T, path string, names []string) []*docir.Docu } plans = append(plans, plan) } - documents, err := docir.EvaluateSet(plans, + outcomes, err := docir.EvaluateSet(plans, queryexec.Context{Index: fixture.index, Resolver: fixture.resolver, Model: fixture.model}, queryexec.Options{}, nil) if err != nil { t.Fatalf("evaluate document set: %v", err) } + documents := make([]*docir.Document, 0, len(outcomes)) + for _, outcome := range outcomes { + if outcome.Err != nil { + t.Fatalf("evaluate document %s: %v", outcome.Name, outcome.Err) + } + documents = append(documents, outcome.Document) + } return documents } diff --git a/internal/doc/docrender/html.go b/internal/doc/docrender/html.go index 5f5ad95632..b7c991c371 100644 --- a/internal/doc/docrender/html.go +++ b/internal/doc/docrender/html.go @@ -175,6 +175,11 @@ type HTMLOptions struct { // does: left undrawn when empty, or drawn too (a strip below a DOT drawing). Unplaced view.Unplaced + // Files is the file each document of the set this one is rendered in is + // written to, by qualified name; a cross-document reference links to the + // target's file here, or to DocumentHTMLFileName of its name when absent. + Files map[string]string + // DiagramImages are images drawn ahead of the render, one per graph-shaped // diagram in the order Diagrams lists them, each written as in place // of its source; an empty entry, or none, keeps the source. More entries @@ -774,31 +779,17 @@ func (w *htmlWriter) runHTML(run docir.TextRun) string { // A scheme a document must not navigate to is kept as data, not as a link. return "" + htmlText(run.Text()) + "" case docir.RunRef: - return "" + htmlText(run.Text()) + "" default: return htmlText(run.Text()) } } -// htmlRefDestination maps a reference run to its destination: an in-document -// anchor, or a relative link into another document's HTML file. -func htmlRefDestination(run docir.TextRun) string { - if run.TargetDocument() == "" { - return "#" + run.Target() - } - destination := DocumentHTMLFileName(run.TargetDocument()) - if run.Target() != "" { - destination += "#" + run.Target() - } - return destination -} - -// DocumentHTMLFileName derives the deterministic HTML file name of a rendered -// document from its fully-qualified name, escaped as anchors are so distinct -// documents never collide. +// DocumentHTMLFileName is the HTML file a document is written to on its own, +// DocumentFileStem of its qualified name plus `.html`. func DocumentHTMLFileName(fqn string) string { - return documentFileName(fqn, ".html") + return DocumentFileStem(fqn) + ".html" } // contentIDs maps every content node's occurrence path to the identifier the diff --git a/internal/doc/docrender/markdown.go b/internal/doc/docrender/markdown.go index 1d6a7d8b07..11ccddf560 100644 --- a/internal/doc/docrender/markdown.go +++ b/internal/doc/docrender/markdown.go @@ -3,6 +3,7 @@ package docrender import ( + "fmt" "slices" "strconv" "strings" @@ -11,6 +12,7 @@ import ( "github.com/Open-MBEE/OpenSysML/internal/doc/queryexec" "github.com/Open-MBEE/OpenSysML/internal/ir/view" "github.com/Open-MBEE/OpenSysML/internal/semantic/symbols" + "github.com/Open-MBEE/OpenSysML/internal/translate/filename" ) // elementColumn heads the single column of a table whose query projected no @@ -27,6 +29,11 @@ type MarkdownOptions struct { // Unplaced is where a diagram some Layout positions puts the nodes none // does: left undrawn when empty, or drawn too (a strip below a DOT drawing). Unplaced view.Unplaced + + // Files is the file each document of the set this one is rendered in is + // written to, by qualified name; a cross-document reference links to the + // target's file here, or to DocumentFileName of its name when absent. + Files map[string]string } // Markdown renders an evaluated document as deterministic CommonMark: the @@ -46,7 +53,7 @@ func Markdown(document *docir.Document, opts MarkdownOptions) (string, error) { if err != nil { return "", err } - w := &markdownWriter{form: form, unplaced: opts.Unplaced} + w := &markdownWriter{form: form, unplaced: opts.Unplaced, files: opts.Files} var blocks []string blocks = append(blocks, heading(1, document.Title())) for _, node := range document.Content() { @@ -76,6 +83,7 @@ func diagramForm(form view.Form) (view.Form, error) { type markdownWriter struct { form view.Form unplaced view.Unplaced + files map[string]string } // figureOptions is what a diagram's rendering is written with: its stated @@ -113,13 +121,13 @@ func (w *markdownWriter) renderNode(node docir.Content, level int) ([]string, er } return blocks, nil case docir.ContentParagraph: - return []string{blockText(node.Runs())}, nil + return []string{w.blockText(node.Runs())}, nil case docir.ContentTable: return renderTable(node), nil case docir.ContentList: - return renderList(node), nil + return w.renderList(node), nil case docir.ContentDefinitions: - return renderDefinitions(node), nil + return w.renderDefinitions(node), nil case docir.ContentFormula: return renderFormula(node), nil case docir.ContentDiagram: @@ -264,7 +272,7 @@ func writeTableRow(b *strings.Builder, cells []string) { // renderList writes one bullet or numbered list, one item per query row. An // empty list renders as nothing, which is the valid Markdown for no items. -func renderList(node docir.Content) []string { +func (w *markdownWriter) renderList(node docir.Content) []string { items := node.Items() if len(items) == 0 { return nil @@ -275,7 +283,7 @@ func renderList(node docir.Content) []string { if node.Style() == docir.ListNumber { marker = strconv.Itoa(i+1) + "." } - lines = append(lines, marker+" "+itemText(item.Runs())) + lines = append(lines, marker+" "+w.itemText(item.Runs())) } return []string{strings.Join(lines, "\n")} } @@ -361,11 +369,11 @@ const definitionSeparator = " — " // renderDefinitions writes one paragraph per entry: the term in strong // emphasis, an em dash, then the description. An entry lacking one side // writes the other alone; one lacking both, like an empty block, writes nothing. -func renderDefinitions(node docir.Content) []string { +func (w *markdownWriter) renderDefinitions(node docir.Content) []string { var blocks []string for _, entry := range node.Definitions() { term := strings.TrimSpace(strongText(entry.Term())) - description := strings.TrimSpace(itemText(entry.Description())) + description := strings.TrimSpace(w.itemText(entry.Description())) switch { case term == "" && description == "": continue @@ -391,23 +399,23 @@ func strongText(runs []docir.TextRun) string { // blockText renders a paragraph's runs joined by single spaces, escaped so the // first character cannot open a heading, list, or quote. -func blockText(runs []docir.TextRun) string { - return blockStart(itemText(runs)) +func (w *markdownWriter) blockText(runs []docir.TextRun) string { + return blockStart(w.itemText(runs)) } // itemText joins text runs by single spaces, rendering each by its kind: // plain runs as escaped prose, styled runs in emphasis or strong delimiters // or as code spans, math runs as dollar math, links and references as inline // links. -func itemText(runs []docir.TextRun) string { +func (w *markdownWriter) itemText(runs []docir.TextRun) string { parts := make([]string, len(runs)) for i, run := range runs { - parts[i] = runText(run) + parts[i] = w.runText(run) } return strings.Join(parts, " ") } -func runText(run docir.TextRun) string { +func (w *markdownWriter) runText(run docir.TextRun) string { switch run.Kind() { case docir.RunEmphasis: return delimited("*", run.Text()) @@ -420,7 +428,7 @@ func runText(run docir.TextRun) string { case docir.RunLink: return "[" + inline(run.Text()) + "](<" + destination(run.Target()) + ">)" case docir.RunRef: - return "[" + inline(run.Text()) + "](" + refDestination(run) + ")" + return "[" + inline(run.Text()) + "](" + w.refDestination(run) + ")" default: return inline(run.Text()) } @@ -428,28 +436,46 @@ func runText(run docir.TextRun) string { // refDestination maps a reference run to its Markdown destination: an // in-document anchor, or a relative link into another document's file. -func refDestination(run docir.TextRun) string { +func (w *markdownWriter) refDestination(run docir.TextRun) string { + return refDestination(run, w.files, DocumentFileName) +} + +// refDestination is a reference run's destination: its anchor within the +// document, or the target document's file, the set's or the default, and anchor. +func refDestination(run docir.TextRun, files map[string]string, defaultFile func(string) string) string { if run.TargetDocument() == "" { return "#" + run.Target() } - destination := DocumentFileName(run.TargetDocument()) + destination, ok := files[run.TargetDocument()] + if !ok { + destination = defaultFile(run.TargetDocument()) + } if run.Target() != "" { destination += "#" + run.Target() } return destination } -// DocumentFileName derives the deterministic Markdown file name of a rendered -// document from its fully-qualified name, using the same escaping as anchors -// so distinct documents never collide. +// DocumentFileName is the Markdown file a document is written to on its own, +// DocumentFileStem of its qualified name plus `.md`. func DocumentFileName(fqn string) string { - return documentFileName(fqn, ".md") + return DocumentFileStem(fqn) + ".md" } -// documentFileName derives a rendered document's file name in one backend's -// extension, escaped as anchors are. -func documentFileName(fqn, extension string) string { - return docir.AnchorFor(strings.Split(fqn, "::")) + extension +// DocumentFileStem is the file name, extension aside, a document derives from +// its qualified name: `::` as `-` and every byte outside ASCII letters, digits +// and `_` as `.XX`, the encoding of anchors. A stem naming a Windows device +// has its first byte encoded too, and one opening with `.` is prefixed `_`, so +// the file is neither a device nor hidden. A set fits and tags the stems it writes. +func DocumentFileStem(fqn string) string { + stem := docir.AnchorFor(strings.Split(fqn, "::")) + if filename.DeviceStem(stem) { + stem = fmt.Sprintf(".%02X", stem[0]) + stem[1:] + } + if strings.HasPrefix(stem, ".") { + stem = "_" + stem + } + return stem } // captionBlock writes a caption as an emphasized paragraph without its surrounding diff --git a/internal/frontend/repl/docrender.go b/internal/frontend/repl/docrender.go index 7eb6894adc..ea0aa999a3 100644 --- a/internal/frontend/repl/docrender.go +++ b/internal/frontend/repl/docrender.go @@ -1,6 +1,7 @@ package repl import ( + "errors" "fmt" "slices" "sort" @@ -11,7 +12,9 @@ import ( "github.com/Open-MBEE/OpenSysML/internal/doc/queryexec" "github.com/Open-MBEE/OpenSysML/internal/ir/docplan" "github.com/Open-MBEE/OpenSysML/internal/ir/view" + "github.com/Open-MBEE/OpenSysML/internal/semantic/semantics" "github.com/Open-MBEE/OpenSysML/internal/semantic/symbols" + "github.com/Open-MBEE/OpenSysML/internal/translate/filename" "github.com/Open-MBEE/OpenSysML/internal/workspace/model" ) @@ -22,20 +25,23 @@ const renderDocumentUsage = "usage: %render-document [mermaid|dot|plantum // RenderDocumentMarkdown compiles the named document definition, evaluates its // queries against the session's model, and renders the result as Markdown. A // document binds its queries' parameters in the model, so the invocation is -// the document's name alone. +// the document's name alone. Its cross-document links point at the files a +// set of the model's documents writes. func (s *Session) RenderDocumentMarkdown(invocation string, opts docrender.MarkdownOptions) (string, error) { defer s.enter()() return s.renderDocumentMarkdown(invocation, opts) } // RenderDocumentHTML compiles the named document definition, evaluates its -// queries against the session's model, and renders the result as HTML. +// queries against the session's model, and renders the result as HTML, its +// cross-document links pointing at the files a set of the model's documents writes. func (s *Session) RenderDocumentHTML(invocation string, opts docrender.HTMLOptions) (string, error) { defer s.enter()() - document, err := s.evaluateDocument(invocation) + document, files, err := s.evaluateDocument(invocation, ".html") if err != nil { return "", err } + opts.Files = files return docrender.HTML(document, opts) } @@ -43,80 +49,104 @@ func (s *Session) RenderDocumentHTML(invocation string, opts docrender.HTMLOptio // queries against the session's model, for a backend rendering the result. func (s *Session) EvaluateDocument(invocation string) (*docir.Document, error) { defer s.enter()() - return s.evaluateDocument(invocation) + document, _, err := s.evaluateDocument(invocation, "") + return document, err } func (s *Session) renderDocumentMarkdown(invocation string, opts docrender.MarkdownOptions) (string, error) { - document, err := s.evaluateDocument(invocation) + document, files, err := s.evaluateDocument(invocation, ".md") if err != nil { return "", err } + opts.Files = files return docrender.Markdown(document, opts) } // evaluateDocument compiles the named document and evaluates it against the // session's model, linked with its siblings so cross-document references -// resolve. -func (s *Session) evaluateDocument(invocation string) (*docir.Document, error) { +// resolve, and plans the files a set of the model's documents in the form of +// extension writes, none when extension is empty. +func (s *Session) evaluateDocument(invocation, extension string) (*docir.Document, map[string]string, error) { fields := splitQueryArgs(strings.TrimSpace(invocation)) if len(fields) == 0 { - return nil, fmt.Errorf("a document to render must be named") + return nil, nil, fmt.Errorf("a document to render must be named") } if len(fields) > 1 { - return nil, fmt.Errorf("a document binds its queries' parameters in the model; unexpected argument %q", fields[1]) + return nil, nil, fmt.Errorf("a document binds its queries' parameters in the model; unexpected argument %q", fields[1]) } sym, fqn, err := s.lookupSymbol(fields[0]) if err != nil { - return nil, err + return nil, nil, err } ctx, err := s.getOrCreateRuntime() if err != nil { - return nil, fmt.Errorf("runtime init: %w", err) + return nil, nil, fmt.Errorf("runtime init: %w", err) } idx := s.browseIndex() sem, resolver := ctx.Semantics(), ctx.Resolver() if !docplan.IsDocumentDefinition(idx, sem, sym) { - return nil, fmt.Errorf("%s is not a document: one is a part def specializing DocumentQueries::Document", notationName(fqn)) + return nil, nil, fmt.Errorf("%s is not a document: one is a part def specializing DocumentQueries::Document", notationName(fqn)) } plan, err := docplan.Compile(idx, sem, resolver, sym) if err != nil { - return nil, err + return nil, nil, err + } + var files map[string]string + if extension != "" { + var names []string + for _, sibling := range s.documentSymbols(idx, sem) { + names = append(names, symbols.FQNOf(sibling)) + } + if files, err = documentFiles(names, extension); err != nil { + return nil, nil, err + } } - return docir.EvaluateLinked(plan, + document, err := docir.EvaluateLinked(plan, model.SiblingDocumentPlans(idx, sem, resolver, sym), s.queryContext(ctx), queryexec.Options{}, s.sessionSourceText()) + return document, files, err } // RenderedDocument is one document of a rendered multi-document set, rendered -// in one backend's form. +// in one backend's form. Err is the error that kept the document from +// rendering, whose Content then is a page stating it; nil for a rendered document. type RenderedDocument struct { Name string FileName string Content string + Err error } // RenderDocumentSetMarkdown compiles every document definition the session's // model declares, evaluates them together, and renders each as Markdown with -// its deterministic file name, so cross-document references link on disk. +// its deterministic file name, so cross-document references link on disk. A +// document that cannot be rendered is a page stating why, under Err, so the +// links into it land on the reason; the others are rendered whole. func (s *Session) RenderDocumentSetMarkdown(opts docrender.MarkdownOptions) ([]RenderedDocument, error) { - return s.renderDocumentSet(docrender.DocumentFileName, - func(document *docir.Document) (string, error) { return docrender.Markdown(document, opts) }) + return s.renderDocumentSet(".md", func(document *docir.Document, files map[string]string) (string, error) { + opts.Files = files + return docrender.Markdown(document, opts) + }) } // RenderDocumentSetHTML renders the same set as linked HTML files, each // referring to the others by their .html file names. func (s *Session) RenderDocumentSetHTML(opts docrender.HTMLOptions) ([]RenderedDocument, error) { - return s.renderDocumentSet(docrender.DocumentHTMLFileName, - func(document *docir.Document) (string, error) { return docrender.HTML(document, opts) }) + return s.renderDocumentSet(".html", func(document *docir.Document, files map[string]string) (string, error) { + opts.Files = files + return docrender.HTML(document, opts) + }) } // renderDocumentSet evaluates every declared document together and renders -// each with one backend, naming its file as that backend does. +// each with one backend into the file planned for it under extension. A +// document that fails to compile, evaluate or render is rendered as the page +// stating its error, which is the document's Err. func (s *Session) renderDocumentSet( - fileName func(string) string, - render func(*docir.Document) (string, error), + extension string, + render func(*docir.Document, map[string]string) (string, error), ) ([]RenderedDocument, error) { defer s.enter()() ctx, err := s.getOrCreateRuntime() @@ -125,55 +155,90 @@ func (s *Session) renderDocumentSet( } idx := s.browseIndex() sem, resolver := ctx.Semantics(), ctx.Resolver() - syms := s.symbolsInLoadOrder(func(scope *symbols.Scope) []*symbols.Symbol { - return model.DeclaredDocumentDefinitions(idx, sem, scope) - }) - sort.SliceStable(syms, func(i, j int) bool { - return symbols.FQNOf(syms[i]) < symbols.FQNOf(syms[j]) - }) + syms := s.documentSymbols(idx, sem) + names := make([]string, 0, len(syms)) + for _, sym := range syms { + names = append(names, symbols.FQNOf(sym)) + } + files, err := documentFiles(names, extension) + if err != nil { + return nil, err + } + failed := map[string]error{} + titles := map[string]string{} plans := make([]*docplan.Plan, 0, len(syms)) - names := map[string]bool{} - // Filenames are compared case-folded so the set stays writable on - // case-insensitive filesystems. - files := map[string]string{} for _, sym := range syms { plan, err := docplan.Compile(idx, sem, resolver, sym) if err != nil { - return nil, err + failed[symbols.FQNOf(sym)] = err + continue } - if names[plan.Name()] { - return nil, fmt.Errorf("%s names more than one document; rename one so the name is unambiguous", notationName(plan.Name())) - } - names[plan.Name()] = true - file := fileName(plan.Name()) - if other, ok := files[strings.ToLower(file)]; ok { - return nil, fmt.Errorf("%s and %s render to file names that differ only by letter case; rename one so both files can coexist on a case-insensitive filesystem", notationName(other), notationName(plan.Name())) - } - files[strings.ToLower(file)] = plan.Name() + titles[plan.Name()] = plan.Title() plans = append(plans, plan) } - documents, err := docir.EvaluateSet(plans, + outcomes, err := docir.EvaluateSet(plans, s.queryContext(ctx), queryexec.Options{}, s.sessionSourceText()) if err != nil { return nil, err } - out := make([]RenderedDocument, 0, len(documents)) - for _, document := range documents { - rendered, err := render(document) - if err != nil { - return nil, err + documents := map[string]*docir.Document{} + for _, outcome := range outcomes { + if outcome.Err != nil { + failed[outcome.Name] = outcome.Err + continue + } + documents[outcome.Name] = outcome.Document + } + out := make([]RenderedDocument, 0, len(names)) + for _, name := range names { + rendered := RenderedDocument{Name: name, FileName: files[name]} + if document, ok := documents[name]; ok { + rendered.Content, rendered.Err = render(document, files) + } else { + rendered.Err = failed[name] } - out = append(out, RenderedDocument{ - Name: document.Name(), - FileName: fileName(document.Name()), - Content: rendered, - }) + if rendered.Err != nil { + rendered.Content, err = render(docir.Unrendered(name, titles[name], rendered.Err), files) + if err != nil { + return nil, err + } + } + out = append(out, rendered) } return out, nil } +// documentSymbols is every document definition the session's model declares, +// in fully-qualified-name order. +func (s *Session) documentSymbols(idx *symbols.Index, sem *semantics.Model) []*symbols.Symbol { + syms := s.symbolsInLoadOrder(func(scope *symbols.Scope) []*symbols.Symbol { + return model.DeclaredDocumentDefinitions(idx, sem, scope) + }) + sort.SliceStable(syms, func(i, j int) bool { + return symbols.FQNOf(syms[i]) < symbols.FQNOf(syms[j]) + }) + return syms +} + +// documentFiles plans the file each named document is written to in the form of +// extension: its stem cut to fit, and tagged with a hash of the whole where two +// would meet letter case aside. Two documents of one name cannot be told apart. +func documentFiles(names []string, extension string) (map[string]string, error) { + files, err := filename.Plan(names, func(name string, tagged bool) string { + return filename.Fit(docrender.DocumentFileStem(name), extension, tagged) + }) + var collision *filename.CollisionError + if errors.As(err, &collision) { + if collision.Names[0] == collision.Names[1] { + return nil, fmt.Errorf("%s names more than one document; rename one so the name is unambiguous", notationName(collision.Names[0])) + } + return nil, fmt.Errorf("%s and %s render to one file name %s; rename one so both files can coexist", notationName(collision.Names[0]), notationName(collision.Names[1]), collision.File) + } + return files, err +} + // doRenderDocument carries out %render-document, printing the rendered // Markdown or reporting a document that could not be rendered. A second // word names the form its graph-shaped diagrams are written in. diff --git a/internal/translate/filename/filename.go b/internal/translate/filename/filename.go new file mode 100644 index 0000000000..e5d56177d0 --- /dev/null +++ b/internal/translate/filename/filename.go @@ -0,0 +1,133 @@ +// Package filename fits the file names a run derives from qualified names +// into what every common filesystem takes, and keeps a set of them apart: +// a name too long for one path component, or whose stem a filesystem reads +// as a device, is cut and tagged with a hash of the whole, and names that +// meet letter case aside are tagged until no two meet. +package filename + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + "strings" + "unicode" + "unicode/utf8" +) + +// Max is the longest name every common filesystem takes for one path component; +// TagBytes of the encoded name's hash keep a cut name apart from its neighbours. +const ( + Max = 255 + TagBytes = 8 +) + +// Fit is name+ext, cut to Max bytes and tagged with `~` and a hash of the whole +// name when tagged, when it is too long, or when its stem is a device name. +// The cut splits neither a UTF-8 sequence nor a trailing `%XX` or `.XX` escape. +func Fit(name, ext string, tagged bool) string { + if tagged || len(name)+len(ext) > Max || DeviceStem(name) { + sum := sha256.Sum256([]byte(name)) + tag := "~" + hex.EncodeToString(sum[:TagBytes]) + name = cut(name, Max-len(ext)-len(tag)) + tag + } + return name + ext +} + +// cut is the longest prefix of name within n bytes that splits neither a +// UTF-8 sequence nor a three-byte escape opened by `%` or `.`. +func cut(name string, n int) string { + n = min(n, len(name)) + for n > 0 && n < len(name) && !utf8.RuneStart(name[n]) { + n-- + } + if i := strings.LastIndexAny(name[:n], "%."); i >= 0 && i > n-3 { + n = i + } + return name[:n] +} + +// DeviceStem reports whether the stem of name, what precedes its first `.`, +// is one Windows reads as a device whatever the extension, trailing spaces +// and letter case aside. +func DeviceStem(name string) bool { + stem, _, _ := strings.Cut(name, ".") + return windowsDeviceNames[strings.ToUpper(strings.TrimRight(stem, " "))] +} + +// windowsDeviceNames are the stems Windows reads as devices whatever the extension, +// trailing spaces and letter case aside: the serial and printer ports include the +// superscript digits Windows counts among them. +var windowsDeviceNames = map[string]bool{ + "CON": true, "PRN": true, "AUX": true, "NUL": true, + "COM0": true, "COM1": true, "COM2": true, "COM3": true, "COM4": true, "COM5": true, "COM6": true, "COM7": true, "COM8": true, "COM9": true, + "COM¹": true, "COM²": true, "COM³": true, + "LPT0": true, "LPT1": true, "LPT2": true, "LPT3": true, "LPT4": true, "LPT5": true, "LPT6": true, "LPT7": true, "LPT8": true, "LPT9": true, + "LPT¹": true, "LPT²": true, "LPT³": true, +} + +// CollisionError reports two names whose files meet even when tagged, which +// only names that are the same, or differ in bytes the encoding drops, can. +type CollisionError struct { + Names [2]string + File string +} + +func (e *CollisionError) Error() string { + return fmt.Sprintf("%s and %s have the same file name %s", e.Names[0], e.Names[1], e.File) +} + +// Plan is the file each name is written to, by name: file gives a name's file, +// tagged or not, and the files that meet letter case aside are tagged until no +// two meet. Two names whose tagged files still meet are a CollisionError. +func Plan(names []string, file func(name string, tagged bool) string) (map[string]string, error) { + tagged := make(map[string]bool, len(names)) + for { + meeting := map[string][]string{} + var keys []string + for _, name := range names { + key := CaseFolded(file(name, tagged[name])) + if _, seen := meeting[key]; !seen { + keys = append(keys, key) + } + meeting[key] = append(meeting[key], name) + } + progressed := false + for _, key := range keys { + group := meeting[key] + if len(group) < 2 { + continue + } + settled := true + for _, name := range group { + if !tagged[name] { + tagged[name], settled, progressed = true, false, true + } + } + if settled { + return nil, &CollisionError{Names: [2]string{group[0], group[1]}, File: file(group[0], true)} + } + } + if !progressed { + break + } + } + files := make(map[string]string, len(names)) + for _, name := range names { + files[name] = file(name, tagged[name]) + } + return files, nil +} + +// CaseFolded is text under simple Unicode case folding, the key two files of a +// Plan meet under: two texts fold alike exactly when strings.EqualFold holds of them. +func CaseFolded(text string) string { + var b strings.Builder + for _, r := range text { + least := r + for f := unicode.SimpleFold(r); f != r; f = unicode.SimpleFold(f) { + least = min(least, f) + } + b.WriteRune(least) + } + return b.String() +} diff --git a/internal/translate/filename/filename_test.go b/internal/translate/filename/filename_test.go new file mode 100644 index 0000000000..3ed21e8712 --- /dev/null +++ b/internal/translate/filename/filename_test.go @@ -0,0 +1,112 @@ +package filename + +import ( + "errors" + "strings" + "testing" +) + +// A name that fits is written as it is; one past Max bytes is cut and tagged +// with a hash of the whole so two long names sharing a prefix stay apart, +// and the cut splits neither a UTF-8 sequence nor a trailing escape. +func TestFitCutsAndTagsLongNames(t *testing.T) { + if got := Fit("Report", ".md", false); got != "Report.md" { + t.Errorf("Fit(Report) = %q, want Report.md", got) + } + long := strings.Repeat("a", 300) + got := Fit(long, ".md", false) + if len(got) != Max || !strings.HasSuffix(got, ".md") || !strings.Contains(got, "~") { + t.Errorf("Fit(long) = %q (%d bytes), want %d bytes, tagged, ending in .md", got, len(got), Max) + } + if other := Fit(long+"b", ".md", false); other == got { + t.Errorf("two long names sharing a prefix are both written to %q", got) + } + tagged := Fit("Report", ".md", true) + if !strings.HasPrefix(tagged, "Report~") || len(tagged) != len("Report~")+2*TagBytes+len(".md") { + t.Errorf("Fit(Report, tagged) = %q, want Report~ and a %d-byte hash", tagged, 2*TagBytes) + } + budget := Max - len(".md") - len("~") - 2*TagBytes + for _, name := range []string{ + strings.Repeat("é", 150), + strings.Repeat("a", budget-1) + ".20" + strings.Repeat("b", 30), + strings.Repeat("a", budget-1) + "%20" + strings.Repeat("b", 30), + } { + got := Fit(name, ".md", false) + stem := got[:strings.LastIndexByte(got, '~')] + if !strings.HasPrefix(name, stem) || strings.HasSuffix(stem, "%") || strings.HasSuffix(stem, ".") || strings.HasSuffix(stem, "%2") || strings.HasSuffix(stem, ".2") { + t.Errorf("Fit(%q) = %q cuts inside a sequence or escape", name[:10]+"…", got) + } + } +} + +// A stem Windows reads as a device is tagged whatever its case, extension or +// trailing spaces, so the file is a file; other stems are left alone. +func TestFitTagsDeviceStems(t *testing.T) { + for _, name := range []string{"CON", "con", "Con ", "nul.report", "COM1", "LPT¹"} { + if !DeviceStem(name) { + t.Errorf("DeviceStem(%q) = false, want true", name) + } + if got := Fit(name, ".md", false); !strings.Contains(got, "~") { + t.Errorf("Fit(%q) = %q, want tagged", name, got) + } + } + for _, name := range []string{"CONSOLE", "COM10", "Report.con", "%43ON"} { + if DeviceStem(name) { + t.Errorf("DeviceStem(%q) = true, want false", name) + } + } +} + +// Plan tags every name whose file meets another's letter case aside, a name +// whose plain file is another's tagged file is tagged in turn, a name that +// meets none keeps its plain file, and two names still meeting tagged are refused. +func TestPlanKeepsFilesApart(t *testing.T) { + file := func(name string, tagged bool) string { return Fit(name, ".md", tagged) } + tagged := file("Report", true) + tagName := strings.TrimSuffix(tagged, ".md") + got, err := Plan([]string{"Report", "report", tagName, "other"}, file) + if err != nil { + t.Fatal(err) + } + if got["Report"] != tagged || got["other"] != "other.md" { + t.Errorf("files = %v, want Report as %s and other plain", got, tagged) + } + if got[tagName] == tagged || !strings.Contains(got[tagName], "~") { + t.Errorf("%s named like the tag is written to %q, want a tagged file other than %s", tagName, got[tagName], tagged) + } + folded := map[string]string{} + for name, f := range got { + if other, met := folded[CaseFolded(f)]; met { + t.Errorf("%s and %s are both written to %s", other, name, f) + } + folded[CaseFolded(f)] = name + } + _, err = Plan([]string{"Report", "Report"}, file) + var collision *CollisionError + if !errors.As(err, &collision) || collision.File != tagged { + t.Errorf("one name twice err = %v, want a CollisionError on %s", err, tagged) + } +} + +// Case folding is Unicode's simple folding, which strings.EqualFold decides: +// a final sigma folds with a sigma, a Kelvin sign with a k, and a name that +// differs in more than case folds apart. +func TestCaseFoldedAgreesWithEqualFold(t *testing.T) { + for _, tc := range []struct { + a, b string + want bool + }{ + {"Report.dot", "report.dot", true}, + {"σ", "ς", true}, + {"Σ", "ς", true}, + {"k", "\u212a", true}, + {"ß", "ẞ", true}, + {"ſ", "S", true}, + {"i", "İ", false}, + {"Report.dot", "Reports.dot", false}, + } { + if got := CaseFolded(tc.a) == CaseFolded(tc.b); got != tc.want || got != strings.EqualFold(tc.a, tc.b) { + t.Errorf("CaseFolded(%q) == CaseFolded(%q) is %v, want %v, as EqualFold says %v", tc.a, tc.b, got, tc.want, strings.EqualFold(tc.a, tc.b)) + } + } +} diff --git a/packaging/man/man1/sysml.1 b/packaging/man/man1/sysml.1 index bc4535b05e..979ffe7f30 100644 --- a/packaging/man/man1/sysml.1 +++ b/packaging/man/man1/sysml.1 @@ -266,7 +266,8 @@ Compile this document definition, run its queries and write the rendered document .TP .BR \-render\-documents " \fIdir\fP" -Render every document definition, linked to one another, into this directory +Render every document definition, linked to one another, into this directory; +a document that cannot be rendered gets a page stating why and the run exits 3 .TP .BR \-doc\-form " \fIform\fP" Form the documents are written in: markdown (default), html or pdf, which @@ -904,6 +905,12 @@ The model answered false for a check. .B 2 What was asked could not be carried out at all \(em an unreadable file, a model that did not analyse cleanly, an unresolved name, a failed conversion. +.TP +.B 3 +Part of what was asked was carried out: a \-render\-documents set in which +some document could not be rendered. The others were written, a page stating +the error stands in for each that was not, and each failure is reported with +the document's qualified name. .SH OUTPUT STREAMS What was asked for is reported on stdout and what went wrong on stderr, prefixed "sysml: " unless it locates a finding in the source. diff --git a/tests/hygiene/layering_test.go b/tests/hygiene/layering_test.go index 47673af954..0532e6cf42 100644 --- a/tests/hygiene/layering_test.go +++ b/tests/hygiene/layering_test.go @@ -84,6 +84,7 @@ var packageLayer = map[string]string{ "internal/translate/rdf/ontology": "translate", "internal/translate/convert": "translate", "internal/translate/export": "translate", + "internal/translate/filename": "translate", "internal/translate/migrate": "translate", "internal/translate/mtip": "translate", "internal/translate/simresults": "translate", From b0d68b98d6ac1155edcd43559564c6ae5b33fd69 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:16:33 +0000 Subject: [PATCH 4/7] fix(cli): cut a long rendering name only at its own encoding's escapes Co-Authored-By: jason.han --- cmd/sysml/render.go | 2 +- internal/frontend/repl/docrender.go | 2 +- internal/translate/filename/filename.go | 23 ++++++----- internal/translate/filename/filename_test.go | 43 +++++++++++--------- 4 files changed, 38 insertions(+), 32 deletions(-) diff --git a/cmd/sysml/render.go b/cmd/sysml/render.go index 3756284a08..69fd0fc3b4 100644 --- a/cmd/sysml/render.go +++ b/cmd/sysml/render.go @@ -235,7 +235,7 @@ func renderFilename(name string, form view.Form, tagged bool) string { if filename.DeviceStem(encoded) { encoded = fmt.Sprintf("%%%02X", encoded[0]) + encoded[1:] } - return filename.Fit(encoded, renderExtension(form), tagged) + return filename.Fit(encoded, renderExtension(form), '%', tagged) } // unsafeFilenameBytes are the printable bytes a rendering filename encodes: path separators, diff --git a/internal/frontend/repl/docrender.go b/internal/frontend/repl/docrender.go index ea0aa999a3..0a0b30e810 100644 --- a/internal/frontend/repl/docrender.go +++ b/internal/frontend/repl/docrender.go @@ -227,7 +227,7 @@ func (s *Session) documentSymbols(idx *symbols.Index, sem *semantics.Model) []*s // would meet letter case aside. Two documents of one name cannot be told apart. func documentFiles(names []string, extension string) (map[string]string, error) { files, err := filename.Plan(names, func(name string, tagged bool) string { - return filename.Fit(docrender.DocumentFileStem(name), extension, tagged) + return filename.Fit(docrender.DocumentFileStem(name), extension, '.', tagged) }) var collision *filename.CollisionError if errors.As(err, &collision) { diff --git a/internal/translate/filename/filename.go b/internal/translate/filename/filename.go index e5d56177d0..660120a68b 100644 --- a/internal/translate/filename/filename.go +++ b/internal/translate/filename/filename.go @@ -1,8 +1,8 @@ // Package filename fits the file names a run derives from qualified names // into what every common filesystem takes, and keeps a set of them apart: -// a name too long for one path component, or whose stem a filesystem reads -// as a device, is cut and tagged with a hash of the whole, and names that -// meet letter case aside are tagged until no two meet. +// a name too long for one path component is cut and tagged with a hash of +// the whole, names that meet letter case aside are tagged until no two meet, +// and a stem a filesystem reads as a device is told so its caller can encode it. package filename import ( @@ -22,25 +22,26 @@ const ( ) // Fit is name+ext, cut to Max bytes and tagged with `~` and a hash of the whole -// name when tagged, when it is too long, or when its stem is a device name. -// The cut splits neither a UTF-8 sequence nor a trailing `%XX` or `.XX` escape. -func Fit(name, ext string, tagged bool) string { - if tagged || len(name)+len(ext) > Max || DeviceStem(name) { +// name when tagged or when it is too long. The cut splits neither a UTF-8 +// sequence nor a trailing three-byte escape, `escape` being the byte the +// name's encoding opens one with (`%` in `%2F`). +func Fit(name, ext string, escape byte, tagged bool) string { + if tagged || len(name)+len(ext) > Max { sum := sha256.Sum256([]byte(name)) tag := "~" + hex.EncodeToString(sum[:TagBytes]) - name = cut(name, Max-len(ext)-len(tag)) + tag + name = cut(name, Max-len(ext)-len(tag), escape) + tag } return name + ext } // cut is the longest prefix of name within n bytes that splits neither a -// UTF-8 sequence nor a three-byte escape opened by `%` or `.`. -func cut(name string, n int) string { +// UTF-8 sequence nor a three-byte escape opened by escape. +func cut(name string, n int, escape byte) string { n = min(n, len(name)) for n > 0 && n < len(name) && !utf8.RuneStart(name[n]) { n-- } - if i := strings.LastIndexAny(name[:n], "%."); i >= 0 && i > n-3 { + if i := strings.LastIndexByte(name[:n], escape); i >= 0 && i > n-3 { n = i } return name[:n] diff --git a/internal/translate/filename/filename_test.go b/internal/translate/filename/filename_test.go index 3ed21e8712..aabf8e7580 100644 --- a/internal/translate/filename/filename_test.go +++ b/internal/translate/filename/filename_test.go @@ -8,47 +8,52 @@ import ( // A name that fits is written as it is; one past Max bytes is cut and tagged // with a hash of the whole so two long names sharing a prefix stay apart, -// and the cut splits neither a UTF-8 sequence nor a trailing escape. +// and the cut splits neither a UTF-8 sequence nor a trailing escape of the +// name's encoding, while a byte that is no escape opener there is kept. func TestFitCutsAndTagsLongNames(t *testing.T) { - if got := Fit("Report", ".md", false); got != "Report.md" { + if got := Fit("Report", ".md", '.', false); got != "Report.md" { t.Errorf("Fit(Report) = %q, want Report.md", got) } long := strings.Repeat("a", 300) - got := Fit(long, ".md", false) + got := Fit(long, ".md", '.', false) if len(got) != Max || !strings.HasSuffix(got, ".md") || !strings.Contains(got, "~") { t.Errorf("Fit(long) = %q (%d bytes), want %d bytes, tagged, ending in .md", got, len(got), Max) } - if other := Fit(long+"b", ".md", false); other == got { + if other := Fit(long+"b", ".md", '.', false); other == got { t.Errorf("two long names sharing a prefix are both written to %q", got) } - tagged := Fit("Report", ".md", true) + tagged := Fit("Report", ".md", '.', true) if !strings.HasPrefix(tagged, "Report~") || len(tagged) != len("Report~")+2*TagBytes+len(".md") { t.Errorf("Fit(Report, tagged) = %q, want Report~ and a %d-byte hash", tagged, 2*TagBytes) } budget := Max - len(".md") - len("~") - 2*TagBytes - for _, name := range []string{ - strings.Repeat("é", 150), - strings.Repeat("a", budget-1) + ".20" + strings.Repeat("b", 30), - strings.Repeat("a", budget-1) + "%20" + strings.Repeat("b", 30), + for _, tc := range []struct { + name string + escape byte + }{ + {strings.Repeat("é", 150), '.'}, + {strings.Repeat("a", budget-1) + ".20" + strings.Repeat("b", 30), '.'}, + {strings.Repeat("a", budget-1) + "%20" + strings.Repeat("b", 30), '%'}, } { - got := Fit(name, ".md", false) + got := Fit(tc.name, ".md", tc.escape, false) stem := got[:strings.LastIndexByte(got, '~')] - if !strings.HasPrefix(name, stem) || strings.HasSuffix(stem, "%") || strings.HasSuffix(stem, ".") || strings.HasSuffix(stem, "%2") || strings.HasSuffix(stem, ".2") { - t.Errorf("Fit(%q) = %q cuts inside a sequence or escape", name[:10]+"…", got) + if !strings.HasPrefix(tc.name, stem) || strings.HasSuffix(stem, string(tc.escape)) || strings.HasSuffix(stem, string(tc.escape)+"2") { + t.Errorf("Fit(%q) = %q cuts inside a sequence or escape", tc.name[:10]+"…", got) } } + dotted := strings.Repeat("a", budget-1) + "." + strings.Repeat("b", 30) + if got := Fit(dotted, ".md", '%', false); !strings.HasPrefix(got, dotted[:budget]+"~") { + t.Errorf("Fit(a….b…, escape %%) = %q, want the `.` kept, as it opens no escape", got) + } } -// A stem Windows reads as a device is tagged whatever its case, extension or -// trailing spaces, so the file is a file; other stems are left alone. -func TestFitTagsDeviceStems(t *testing.T) { +// A stem Windows reads as a device is told apart whatever its case, extension +// or trailing spaces, so a caller can encode it; other stems are not. +func TestDeviceStem(t *testing.T) { for _, name := range []string{"CON", "con", "Con ", "nul.report", "COM1", "LPT¹"} { if !DeviceStem(name) { t.Errorf("DeviceStem(%q) = false, want true", name) } - if got := Fit(name, ".md", false); !strings.Contains(got, "~") { - t.Errorf("Fit(%q) = %q, want tagged", name, got) - } } for _, name := range []string{"CONSOLE", "COM10", "Report.con", "%43ON"} { if DeviceStem(name) { @@ -61,7 +66,7 @@ func TestFitTagsDeviceStems(t *testing.T) { // whose plain file is another's tagged file is tagged in turn, a name that // meets none keeps its plain file, and two names still meeting tagged are refused. func TestPlanKeepsFilesApart(t *testing.T) { - file := func(name string, tagged bool) string { return Fit(name, ".md", tagged) } + file := func(name string, tagged bool) string { return Fit(name, ".md", '.', tagged) } tagged := file("Report", true) tagName := strings.TrimSuffix(tagged, ".md") got, err := Plan([]string{"Report", "report", tagName, "other"}, file) From e69eeae9477812ab435c84ee362bbc3dfac1909b Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:21:17 +0000 Subject: [PATCH 5/7] fix(cli): drop a duplicated doc comment on runRenderDocuments Co-Authored-By: jason.han --- cmd/sysml/render_document.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/cmd/sysml/render_document.go b/cmd/sysml/render_document.go index 88c06f4b3b..13a12f30c8 100644 --- a/cmd/sysml/render_document.go +++ b/cmd/sysml/render_document.go @@ -86,9 +86,6 @@ func pdfOptions() (docpdf.Options, error) { }, nil } -// runRenderDocuments renders every document definition of the model named on -// the command line as linked files in the directory -render-documents names, -// so cross-document references resolve on disk. // runRenderDocuments renders every document of the model named on the command // line into -render-documents as a linked set, writing the pages of the // documents that render and, for each that does not, a page stating why; it From 1d544e22beaf30393d4da1c6644974cc58775141 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 24 Sep 2026 23:41:25 +0000 Subject: [PATCH 6/7] fix(doc): carry incoming anchors onto a failed document's page; document null as an empty cell A set's page for a document that could not be rendered now carries a paragraph under each anchor the other documents link into it by, so a link into one of its blocks lands on the page rather than on a fragment that does not exist. A column expression of null, alone or as the default of ??, declares an empty cell; the query cookbook and the changelog fragment say so and a test pins it. Co-Authored-By: jason.han --- .../document-multi-valued-cells.fixed.md | 2 +- cmd/sysml/render_documents_test.go | 24 +++-- docs/manual/outputs.md | 7 +- docs/manual/query-cookbook.md | 3 +- docs/reference/cli.md | 3 +- internal/doc/docir/evaluate.go | 97 ++++++++++++------- internal/doc/queryexec/computed.go | 5 +- internal/doc/queryexec/computed_test.go | 34 +++++++ internal/frontend/repl/docrender.go | 2 +- 9 files changed, 127 insertions(+), 50 deletions(-) diff --git a/changes/unreleased/document-multi-valued-cells.fixed.md b/changes/unreleased/document-multi-valued-cells.fixed.md index 7defd61b17..45fcfc9fd9 100644 --- a/changes/unreleased/document-multi-valued-cells.fixed.md +++ b/changes/unreleased/document-multi-valued-cells.fixed.md @@ -1 +1 @@ -- **A table column over a multi-valued feature renders its values instead of failing the document.** A `Column` whose expression reads a feature declared `[0..*]` (a migrated DocGen table over `attribute :>> tCalibNB = (69.0, 98.0);`) stopped the whole document with "produced 2 values, expected one". The query planner now carries the column's declared multiplicity into execution: a cell holds as many values as the feature admits, written in order and `, `-joined in Markdown, HTML and PDF alike, and an optional feature with no value is an empty cell rather than an error. A column declared `[1]` still refuses zero or several values, naming the bound it expected, and a scalar place — a caption, a `Ref`, a comparison operand — still takes one value. +- **A table column over a multi-valued feature renders its values instead of failing the document.** A `Column` whose expression reads a feature declared `[0..*]` (a migrated DocGen table over `attribute :>> tCalibNB = (69.0, 98.0);`) stopped the whole document with "produced 2 values, expected one". The query planner now carries the column's declared multiplicity into execution: a cell holds as many values as the feature admits, written in order and `, `-joined in Markdown, HTML and PDF alike, and an optional feature with no value is an empty cell rather than an error. A column declared `[1]` still refuses zero or several values, naming the bound it expected, unless `?? null` opts its empty rows into an empty cell; a scalar place — a caption, a `Ref`, a comparison operand — still takes one value. diff --git a/cmd/sysml/render_documents_test.go b/cmd/sysml/render_documents_test.go index 5afd5d122b..0c6d22dd85 100644 --- a/cmd/sysml/render_documents_test.go +++ b/cmd/sysml/render_documents_test.go @@ -571,7 +571,8 @@ func TestRenderDocumentsSameShortName(t *testing.T) { } // partialModel declares three documents, one of which fails to evaluate: its -// table reads a [1] attribute the row leaves unbound. The others link to it. +// table reads a [1] attribute the row leaves unbound. Another links to it and +// to its table. const partialModel = `package Reports { private import DocumentQueries::*; private import KerML::Root::Element; @@ -608,6 +609,9 @@ const partialModel = `package Reports { part see : Ref { ref redefines target = brokenDoc; } + part table : Ref { + ref redefines target = brokenDoc.timings; + } } } @@ -622,16 +626,18 @@ const partialModel = `package Reports { // TestRenderDocumentsPartialSet checks a set with one document that cannot be // rendered still writes the others, writes a page stating the error where the -// failed document's links land, names the failure on stderr, and exits 3. +// failed document's links land — carrying the anchors links into its blocks +// expect — names the failure on stderr, and exits 3. func TestRenderDocumentsPartialSet(t *testing.T) { binary := buildCLI(t) for _, form := range []struct { name, ext string args []string - link string + links []string + anchor string }{ - {"markdown", ".md", nil, "](Reports-Broken.md)"}, - {"html", ".html", []string{"-doc-form", "html"}, `href="Reports-Broken.html"`}, + {"markdown", ".md", nil, []string{"](Reports-Broken.md)", "](Reports-Broken.md#timings)"}, ``}, + {"html", ".html", []string{"-doc-form", "html"}, []string{`href="Reports-Broken.html"`, `href="Reports-Broken.html#timings"`}, `id="timings"`}, } { t.Run(form.name, func(t *testing.T) { dir := filepath.Join(t.TempDir(), "rendered") @@ -657,14 +663,16 @@ func TestRenderDocumentsPartialSet(t *testing.T) { if err != nil { t.Fatal(err) } - if !strings.Contains(string(first), form.link) { - t.Errorf("the link to the failed document is not %s:\n%s", form.link, first) + for _, want := range form.links { + if !strings.Contains(string(first), want) { + t.Errorf("the link to the failed document is not %s:\n%s", want, first) + } } broken, err := os.ReadFile(filepath.Join(dir, "Reports-Broken"+form.ext)) if err != nil { t.Fatal(err) } - for _, want := range []string{"Broken Timings", "This document could not be rendered.", "duration"} { + for _, want := range []string{"Broken Timings", "This document could not be rendered.", "duration", form.anchor, "timings", "was not rendered with the rest of this document."} { if !strings.Contains(string(broken), want) { t.Errorf("the failed document's page lacks %q:\n%s", want, broken) } diff --git a/docs/manual/outputs.md b/docs/manual/outputs.md index a16c449ca3..cace9375c5 100644 --- a/docs/manual/outputs.md +++ b/docs/manual/outputs.md @@ -64,9 +64,10 @@ Each document in the set is compiled and evaluated on its own. One that cannot be rendered — a query column with no value for a row, a diagram past its form's limit — does not stop the others: they are written, a page carrying the document's title, **This document could not be rendered.** and the error -is written in its place so links to it resolve, each failure is reported on -stderr as `document could not be rendered: `, and the -run exits with status 3 rather than 0. +is written in its place so links to it resolve — a link into one of its blocks +lands on a line naming that block, under the anchor the link expects — each +failure is reported on stderr as `document could not be +rendered: `, and the run exits with status 3 rather than 0. ## HTML diff --git a/docs/manual/query-cookbook.md b/docs/manual/query-cookbook.md index f04014a2a2..b1313b81ff 100644 --- a/docs/manual/query-cookbook.md +++ b/docs/manual/query-cookbook.md @@ -745,7 +745,8 @@ fills the cell with both in order (comma-joined in a document table) and one carrying none leaves it empty, while a feature declared without a multiplicity is one value per row — a row binding two fails the column with a typed `column-cardinality` error naming the declared bound, and a row binding none -with `column-absent` unless `??` supplies a default. Operators always take one +with `column-absent` unless `??` supplies a default — a value, or `null` to +leave that row's cell empty (`Stage::mass ?? null`). Operators always take one value per operand, so `Element::documentation + "."` over two bodies fails. Quantities take part in column arithmetic with the runtime's rules, so a diff --git a/docs/reference/cli.md b/docs/reference/cli.md index b46303490d..d3fefdb2cf 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -701,7 +701,8 @@ is written. Once the set is being rendered, each document is compiled and evalua one that fails — a query whose column has no value for a row, a diagram past a form's limit — does not stop the others. The run writes every document that rendered, writes in place of each one that did not a page carrying its title, **This document could not be rendered.** and the error, so -links to it from other pages resolve to that explanation rather than dangling, and then reports +links to it from other pages resolve to that explanation rather than dangling (a link into one of +its blocks lands on a line naming that block, under the anchor the link expects), and then reports each failure on stderr as `document could not be rendered: ` and exits with status 3 (see [Exit status](#exit-status)). `-render-documents` cannot be combined with `-render-document`, `-render`, `-render-all`, `-o`, `-convert`, a query flag, or a check flag diff --git a/internal/doc/docir/evaluate.go b/internal/doc/docir/evaluate.go index f4d76825b7..f8009e0bba 100644 --- a/internal/doc/docir/evaluate.go +++ b/internal/doc/docir/evaluate.go @@ -2,6 +2,7 @@ package docir import ( "fmt" + "sort" "strconv" "strings" @@ -34,13 +35,7 @@ func EvaluateLinked( options queryexec.Options, text view.SourceText, ) (*Document, error) { - external := make(map[string]map[string]bool) - for _, sibling := range siblings { - if sibling.Compiled() { - collectCrossAnchors(sibling.Content(), external) - } - } - return evaluate(plan, context, options, text, external[plan.Name()]) + return evaluate(plan, context, options, text, crossAnchors(siblings)[plan.Name()]) } // Evaluated is the outcome of evaluating one plan of a set: the document, or @@ -62,13 +57,12 @@ func EvaluateSet( options queryexec.Options, text view.SourceText, ) ([]Evaluated, error) { - external := make(map[string]map[string]bool) for _, plan := range plans { if !plan.Compiled() { return nil, &Error{Kind: ErrorInvalidPlan} } - collectCrossAnchors(plan.Content(), external) } + external := crossAnchors(plans) context = sharingRelationshipTables(context) outcomes := make([]Evaluated, 0, len(plans)) for _, plan := range plans { @@ -79,23 +73,50 @@ func EvaluateSet( } // Unrendered is the document a set writes in place of the one named name, -// titled title, that could not be rendered: one paragraph stating why, so a -// link into it lands on the reason rather than on nothing. -func Unrendered(name, title string, err error) *Document { +// titled title, that could not be rendered: a paragraph stating why, then one +// carrying each anchor the siblings' plans link into it by, so every link into +// the document lands on the reason rather than on nothing. +func Unrendered(name, title string, err error, siblings []*docplan.Plan) *Document { if title == "" { title = name } - return &Document{ - name: name, - title: title, - content: []Content{{ - kind: ContentParagraph, + content := []Content{{ + kind: ContentParagraph, + runs: []TextRun{ + {kind: RunStrong, text: "This document could not be rendered."}, + {kind: RunPlain, text: err.Error()}, + }, + }} + for _, anchor := range linkedAnchors(name, siblings) { + content = append(content, Content{ + kind: ContentParagraph, + anchor: anchor.id, runs: []TextRun{ - {kind: RunStrong, text: "This document could not be rendered."}, - {kind: RunPlain, text: err.Error()}, + {kind: RunCode, text: strings.Join(anchor.path, "::")}, + {kind: RunPlain, text: "was not rendered with the rest of this document."}, }, - }}, + }) + } + return &Document{name: name, title: title, content: content} +} + +// linkedAnchor is a content block of one document that another's reference +// run links to: its stable anchor and the named path deriving it. +type linkedAnchor struct { + id string + path []string +} + +// linkedAnchors is every block of the document named name that the compiled +// plans among siblings link into, in anchor order. +func linkedAnchors(name string, siblings []*docplan.Plan) []linkedAnchor { + external := crossAnchors(siblings) + anchors := make([]linkedAnchor, 0, len(external[name])) + for id, path := range external[name] { + anchors = append(anchors, linkedAnchor{id: id, path: path}) } + sort.Slice(anchors, func(i, j int) bool { return anchors[i].id < anchors[j].id }) + return anchors } func evaluate( @@ -103,7 +124,7 @@ func evaluate( context queryexec.Context, options queryexec.Options, text view.SourceText, - external map[string]bool, + external map[string][]string, ) (*Document, error) { if !plan.Compiled() { return nil, &Error{Kind: ErrorInvalidPlan} @@ -145,21 +166,31 @@ func sharingRelationshipTables(context queryexec.Context) queryexec.Context { return context } -// collectCrossAnchors records, per target document, the anchors that other -// documents' reference runs require it to emit. -func collectCrossAnchors(planned []docplan.Content, external map[string]map[string]bool) { - for _, node := range planned { - for _, run := range node.Runs() { - if run.Kind() != docplan.RunRef || run.RefDocument() == "" || len(run.RefPath()) == 0 { - continue - } - if external[run.RefDocument()] == nil { - external[run.RefDocument()] = make(map[string]bool) +// crossAnchors records, per target document, the anchors the compiled plans' +// reference runs require it to emit, each with the named path deriving it. +func crossAnchors(plans []*docplan.Plan) map[string]map[string][]string { + external := make(map[string]map[string][]string) + var collect func(planned []docplan.Content) + collect = func(planned []docplan.Content) { + for _, node := range planned { + for _, run := range node.Runs() { + if run.Kind() != docplan.RunRef || run.RefDocument() == "" || len(run.RefPath()) == 0 { + continue + } + if external[run.RefDocument()] == nil { + external[run.RefDocument()] = make(map[string][]string) + } + external[run.RefDocument()][AnchorFor(run.RefPath())] = run.RefPath() } - external[run.RefDocument()][AnchorFor(run.RefPath())] = true + collect(node.Children()) + } + } + for _, plan := range plans { + if plan.Compiled() { + collect(plan.Content()) } - collectCrossAnchors(node.Children(), external) } + return external } type evaluator struct { diff --git a/internal/doc/queryexec/computed.go b/internal/doc/queryexec/computed.go index 0f7a7eabda..d71c6b8312 100644 --- a/internal/doc/queryexec/computed.go +++ b/internal/doc/queryexec/computed.go @@ -117,7 +117,8 @@ func (e *executor) evaluateColumnCell( // columnMultiplicity is how many values a column expression may produce: what // the feature or parameter it reads declares, one for an operator's result, -// and a literal's own count; `a ?? b` admits either operand's count. +// and a literal's own count — none for `null`, which declares an empty cell; +// `a ?? b` admits either operand's count. func columnMultiplicity(expression queryplan.Expression) queryplan.Multiplicity { one := queryplan.Multiplicity{Lower: 1, Upper: 1, Known: true} switch expression.Operation() { @@ -125,7 +126,7 @@ func columnMultiplicity(expression queryplan.Expression) queryplan.Multiplicity return expression.Multiplicity() case queryplan.OperationLiteral: if kind, _ := expression.Literal(); kind == queryplan.LiteralNull { - return queryplan.Multiplicity{Known: true} + return queryplan.Multiplicity{Lower: 0, Upper: 0, Known: true} } return one case queryplan.OperationColumnOperator: diff --git a/internal/doc/queryexec/computed_test.go b/internal/doc/queryexec/computed_test.go index a625dc3b9e..1088ac1ebf 100644 --- a/internal/doc/queryexec/computed_test.go +++ b/internal/doc/queryexec/computed_test.go @@ -516,6 +516,40 @@ calc def Bad :> Query { } } +// A `null` literal declares an empty cell: on its own, and as the `??` +// default of a `[1]` feature a row leaves unbound. +func TestExecuteComputedNullLiteralIsEmptyCell(t *testing.T) { + fixture := loadExecutionFixture(t, ` +part def Box { + attribute size : Real[1]; +} +part shed { + part b : Box; +} +calc def Blank :> Query { + in root : Element; + Project( + source = Descendants(source = root, maxDepth = 1), + columns = (Column(name = "none", expression = null), Column(name = "s", expression = Box::size ?? null)) + ) +}`) + result, err := fixture.execute(t, "Blank", Bindings{ + "root": {ElementValue(fixture.symbol(t, "shed"))}, + }, Options{}) + if err != nil { + t.Fatalf("execute: %v", err) + } + rows := result.Rows() + if len(rows) != 1 { + t.Fatalf("rows = %d, want one", len(rows)) + } + for i, cell := range rows[0].Cells() { + if len(cell.Values()) != 0 { + t.Fatalf("cell %d = %+v, want an empty cell", i, cell.Values()) + } + } +} + // A `[1]` feature a row leaves unbound is absent, not an empty cell. func TestExecuteComputedRequiredFeatureRejectsNoValue(t *testing.T) { fixture := loadExecutionFixture(t, ` diff --git a/internal/frontend/repl/docrender.go b/internal/frontend/repl/docrender.go index 0a0b30e810..d0a8c11115 100644 --- a/internal/frontend/repl/docrender.go +++ b/internal/frontend/repl/docrender.go @@ -200,7 +200,7 @@ func (s *Session) renderDocumentSet( rendered.Err = failed[name] } if rendered.Err != nil { - rendered.Content, err = render(docir.Unrendered(name, titles[name], rendered.Err), files) + rendered.Content, err = render(docir.Unrendered(name, titles[name], rendered.Err, plans), files) if err != nil { return nil, err } From 7bff0552884c2007df0c7e1d31d2a42c488d0cf1 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 24 Sep 2026 23:54:28 +0000 Subject: [PATCH 7/7] fix(doc): link a document rendered alone by the set's planned file names in every frontend The LSP's opensysml/renderDocument and the gRPC RenderDocument rendered a document without the set's file plan, so a cross-document link pointed at the untagged name where -render-documents writes a tagged one. The planner now lives in workspace/model as DocumentFiles, and the REPL, LSP and gRPC renderers all link through it. Co-Authored-By: jason.han --- .../render-documents-same-name.fixed.md | 2 +- docs/reference/lsp.md | 4 +- internal/frontend/grpc/docquery.go | 12 +++- internal/frontend/grpc/docquery_test.go | 58 +++++++++++++++++++ internal/frontend/repl/docrender.go | 23 +------- internal/workspace/model/docquery.go | 6 +- .../workspace/model/docquery_files_test.go | 51 ++++++++++++++++ internal/workspace/model/selection.go | 38 ++++++++++++ 8 files changed, 168 insertions(+), 26 deletions(-) create mode 100644 internal/workspace/model/docquery_files_test.go diff --git a/changes/unreleased/render-documents-same-name.fixed.md b/changes/unreleased/render-documents-same-name.fixed.md index 69e5b4461c..979cc95892 100644 --- a/changes/unreleased/render-documents-same-name.fixed.md +++ b/changes/unreleased/render-documents-same-name.fixed.md @@ -1 +1 @@ -- **`-render-documents` writes documents whose file names differ in letter case alone instead of stopping.** Two documents `Reports::Summary` and `Reports::SUMMARY` stopped the run with "render to file names that differ only by letter case"; each is now written under its name tagged with `~` and a hash, as `-render-all` writes such views, and the same planner escapes a stem Windows reads as a device and cuts a name too long for a path component. Cross-document links, in a set and in a single document rendered on its own, point at the tagged name where one was needed. Documents sharing a short name in different packages were always written apart, by qualified name, and naming one to `-render-document` by the short name alone is refused with every candidate's qualified name. +- **`-render-documents` writes documents whose file names differ in letter case alone instead of stopping.** Two documents `Reports::Summary` and `Reports::SUMMARY` stopped the run with "render to file names that differ only by letter case"; each is now written under its name tagged with `~` and a hash, as `-render-all` writes such views, and the same planner escapes a stem Windows reads as a device and cuts a name too long for a path component. Cross-document links point at the tagged name where one was needed, in a set and in a single document rendered on its own — by `-render-document`, `%render-document`, the LSP's `opensysml/renderDocument` or the gRPC `RenderDocument`, which all plan the set's file names the same way. Documents sharing a short name in different packages were always written apart, by qualified name, and naming one to `-render-document` by the short name alone is refused with every candidate's qualified name. diff --git a/docs/reference/lsp.md b/docs/reference/lsp.md index b3370781aa..fdb8831e79 100644 --- a/docs/reference/lsp.md +++ b/docs/reference/lsp.md @@ -270,7 +270,9 @@ workspace's own files declare. Renders one document definition to Markdown: the document is compiled to a plan, its queries are executed against the workspace model, and the result is written the way the REPL's `%render-document` and `sysml -render-document` write it. It is -the same pipeline, run against the same workspace the diagnostics are computed from. +the same pipeline, run against the same workspace the diagnostics are computed from, +so a cross-document reference links the file name `sysml -render-documents ` +gives its target, tagged where the set would tag it. ```json { "name": "Observatory::MassReport" } diff --git a/internal/frontend/grpc/docquery.go b/internal/frontend/grpc/docquery.go index 393b00b3d7..b4ce12804d 100644 --- a/internal/frontend/grpc/docquery.go +++ b/internal/frontend/grpc/docquery.go @@ -117,14 +117,22 @@ func (s *Service) RenderDocument(ctx context.Context, req *pb.RenderDocumentRequ if err != nil { return nil, held.documentStatus(err) } + extension := ".md" if form == renderFormHTML { - page, err := docrender.HTML(document, docrender.HTMLOptions{}) + extension = ".html" + } + files, err := model.DocumentFiles(model.DocumentNames(qctx.Index, qctx.Model), extension) + if err != nil { + return nil, documentStatus(err) + } + if form == renderFormHTML { + page, err := docrender.HTML(document, docrender.HTMLOptions{Files: files}) if err != nil { return nil, documentStatus(err) } return &pb.RenderDocumentResponse{Html: page}, nil } - markdown, err := docrender.Markdown(document, docrender.MarkdownOptions{}) + markdown, err := docrender.Markdown(document, docrender.MarkdownOptions{Files: files}) if err != nil { return nil, documentStatus(err) } diff --git a/internal/frontend/grpc/docquery_test.go b/internal/frontend/grpc/docquery_test.go index a13db1b2aa..c85fd93c95 100644 --- a/internal/frontend/grpc/docquery_test.go +++ b/internal/frontend/grpc/docquery_test.go @@ -2,7 +2,9 @@ package grpc import ( "context" + "fmt" "os" + "regexp" "slices" "strings" "testing" @@ -10,6 +12,7 @@ import ( "connectrpc.com/connect" pb "github.com/Open-MBEE/OpenSysML/api/proto" "github.com/Open-MBEE/OpenSysML/internal/doc/queryexec" + "github.com/Open-MBEE/OpenSysML/internal/workspace/model" ) // telescopeFixture is the document pipeline's own telescope-domain fixture, so @@ -575,6 +578,61 @@ func TestRenderDocumentUsesParameterDefaults(t *testing.T) { } } +// A document rendered on its own links a sibling by the file a set of the same +// form writes it to, tag included, in Markdown and in HTML alike. +func TestRenderDocumentLinksSiblingsByPlannedFiles(t *testing.T) { + srv := mustNewService(t, 10) + parsed, err := srv.ParseFile(context.Background(), &pb.ParseFileRequest{ + Source: &pb.ParseFileRequest_Content{Content: `package Reports { + private import DocumentQueries::*; + + ref shouting : WEEKLY; + + part def Weekly :> Document { + attribute redefines title = "Weekly"; + part intro : Paragraph { + part see : Ref { + ref redefines target = shouting; + } + } + } + part def WEEKLY :> Document { + attribute redefines title = "WEEKLY"; + } +} +`}, + }) + if err != nil { + t.Fatalf("ParseFile failed: %v", err) + } + names := []string{"Reports::Weekly", "Reports::WEEKLY"} + for _, form := range []struct { + form, extension, link string + rendered func(*pb.RenderDocumentResponse) string + }{ + {"markdown", ".md", "](%s)", func(r *pb.RenderDocumentResponse) string { return r.Markdown }}, + {"html", ".html", `href="%s"`, func(r *pb.RenderDocumentResponse) string { return r.Html }}, + } { + files, err := model.DocumentFiles(names, form.extension) + if err != nil { + t.Fatalf("DocumentFiles: %v", err) + } + if !regexp.MustCompile(`^Reports-WEEKLY~[0-9a-f]+\` + form.extension + `$`).MatchString(files["Reports::WEEKLY"]) { + t.Fatalf("planned file %q is not tagged", files["Reports::WEEKLY"]) + } + resp, err := srv.RenderDocument(context.Background(), &pb.RenderDocumentRequest{ + ModelHash: parsed.ModelHash, DocumentId: "Reports::Weekly", Form: form.form, + }) + if err != nil { + t.Fatalf("RenderDocument(%s) failed: %v", form.form, err) + } + want := fmt.Sprintf(form.link, files["Reports::WEEKLY"]) + if !strings.Contains(form.rendered(resp), want) { + t.Errorf("%s lacks %q:\n%s", form.form, want, form.rendered(resp)) + } + } +} + func TestRenderDocumentFailures(t *testing.T) { srv := mustNewService(t, 10) hash := parseTelescope(t, srv) diff --git a/internal/frontend/repl/docrender.go b/internal/frontend/repl/docrender.go index d0a8c11115..774feba62f 100644 --- a/internal/frontend/repl/docrender.go +++ b/internal/frontend/repl/docrender.go @@ -1,7 +1,6 @@ package repl import ( - "errors" "fmt" "slices" "sort" @@ -14,7 +13,6 @@ import ( "github.com/Open-MBEE/OpenSysML/internal/ir/view" "github.com/Open-MBEE/OpenSysML/internal/semantic/semantics" "github.com/Open-MBEE/OpenSysML/internal/semantic/symbols" - "github.com/Open-MBEE/OpenSysML/internal/translate/filename" "github.com/Open-MBEE/OpenSysML/internal/workspace/model" ) @@ -97,7 +95,7 @@ func (s *Session) evaluateDocument(invocation, extension string) (*docir.Documen for _, sibling := range s.documentSymbols(idx, sem) { names = append(names, symbols.FQNOf(sibling)) } - if files, err = documentFiles(names, extension); err != nil { + if files, err = model.DocumentFiles(names, extension); err != nil { return nil, nil, err } } @@ -160,7 +158,7 @@ func (s *Session) renderDocumentSet( for _, sym := range syms { names = append(names, symbols.FQNOf(sym)) } - files, err := documentFiles(names, extension) + files, err := model.DocumentFiles(names, extension) if err != nil { return nil, err } @@ -222,23 +220,6 @@ func (s *Session) documentSymbols(idx *symbols.Index, sem *semantics.Model) []*s return syms } -// documentFiles plans the file each named document is written to in the form of -// extension: its stem cut to fit, and tagged with a hash of the whole where two -// would meet letter case aside. Two documents of one name cannot be told apart. -func documentFiles(names []string, extension string) (map[string]string, error) { - files, err := filename.Plan(names, func(name string, tagged bool) string { - return filename.Fit(docrender.DocumentFileStem(name), extension, '.', tagged) - }) - var collision *filename.CollisionError - if errors.As(err, &collision) { - if collision.Names[0] == collision.Names[1] { - return nil, fmt.Errorf("%s names more than one document; rename one so the name is unambiguous", notationName(collision.Names[0])) - } - return nil, fmt.Errorf("%s and %s render to one file name %s; rename one so both files can coexist", notationName(collision.Names[0]), notationName(collision.Names[1]), collision.File) - } - return files, err -} - // doRenderDocument carries out %render-document, printing the rendered // Markdown or reporting a document that could not be rendered. A second // word names the form its graph-shaped diagrams are written in. diff --git a/internal/workspace/model/docquery.go b/internal/workspace/model/docquery.go index 211ac5d1d7..0fa24f6070 100644 --- a/internal/workspace/model/docquery.go +++ b/internal/workspace/model/docquery.go @@ -42,7 +42,8 @@ func (w *Workspace) DocumentDefinitions() []DocumentDefinition { } // RenderDocumentMarkdown compiles the named document definition, evaluates its -// queries against the workspace model, and renders the result as Markdown. +// queries against the workspace model, and renders the result as Markdown, +// linking the other documents by the files a Markdown set writes them to. func (w *Workspace) RenderDocumentMarkdown(fqn string, opts docrender.MarkdownOptions) (string, error) { w.mu.Lock() defer w.mu.Unlock() @@ -65,6 +66,9 @@ func (w *Workspace) RenderDocumentMarkdown(fqn string, opts docrender.MarkdownOp if plan, err = docplan.Compile(w.index, sem, resolver, sym); err != nil { return } + if opts.Files, err = DocumentFiles(DocumentNames(w.index, sem), ".md"); err != nil { + return + } var document *docir.Document document, err = docir.EvaluateLinked(plan, SiblingDocumentPlans(w.index, sem, resolver, sym), diff --git a/internal/workspace/model/docquery_files_test.go b/internal/workspace/model/docquery_files_test.go new file mode 100644 index 0000000000..5d1b11d611 --- /dev/null +++ b/internal/workspace/model/docquery_files_test.go @@ -0,0 +1,51 @@ +package model + +import ( + "regexp" + "testing" + + "github.com/Open-MBEE/OpenSysML/internal/doc/docrender" +) + +// caseCollidingDocumentModel declares two documents whose names meet letter +// case aside, one referring to the other, so a set tags both files. +const caseCollidingDocumentModel = ` +package Reports { + private import DocumentQueries::*; + + ref shouting : WEEKLY; + + part def Weekly :> Document { + attribute redefines title = "Weekly"; + part intro : Paragraph { + part see : Ref { + ref redefines target = shouting; + } + } + } + part def WEEKLY :> Document { + attribute redefines title = "WEEKLY"; + } +} +` + +// A document rendered on its own links a sibling by the file a set writes it +// to, tag included, so a preview's links land on the set's files. +func TestRenderDocumentMarkdownLinksSiblingsByPlannedFiles(t *testing.T) { + ws := openDoc(t, "reports.sysml", caseCollidingDocumentModel) + markdown, err := ws.RenderDocumentMarkdown("Reports::Weekly", docrender.MarkdownOptions{}) + if err != nil { + t.Fatalf("RenderDocumentMarkdown: %v", err) + } + files, err := DocumentFiles([]string{"Reports::Weekly", "Reports::WEEKLY"}, ".md") + if err != nil { + t.Fatalf("DocumentFiles: %v", err) + } + want := regexp.QuoteMeta("](" + files["Reports::WEEKLY"] + ")") + if !regexp.MustCompile(`Reports-WEEKLY~[0-9a-f]+\.md`).MatchString(files["Reports::WEEKLY"]) { + t.Fatalf("planned file %q is not tagged", files["Reports::WEEKLY"]) + } + if !regexp.MustCompile(want).MatchString(markdown) { + t.Errorf("markdown does not link %s:\n%s", files["Reports::WEEKLY"], markdown) + } +} diff --git a/internal/workspace/model/selection.go b/internal/workspace/model/selection.go index 863e3171b2..c5b7a1649b 100644 --- a/internal/workspace/model/selection.go +++ b/internal/workspace/model/selection.go @@ -1,12 +1,17 @@ package model import ( + "errors" + "fmt" "sort" + "github.com/Open-MBEE/OpenSysML/internal/doc/docrender" "github.com/Open-MBEE/OpenSysML/internal/ir/docplan" "github.com/Open-MBEE/OpenSysML/internal/semantic/resolve" "github.com/Open-MBEE/OpenSysML/internal/semantic/semantics" "github.com/Open-MBEE/OpenSysML/internal/semantic/symbols" + "github.com/Open-MBEE/OpenSysML/internal/syntax/source" + "github.com/Open-MBEE/OpenSysML/internal/translate/filename" ) // DeclaredViews returns the views declared in scope and its nested scopes, @@ -33,6 +38,39 @@ func DeclaredDocumentDefinitions(index *symbols.Index, sem *semantics.Model, sco return out } +// DocumentNames is the qualified name of every document definition the +// workspace documents declare, in name order. +func DocumentNames(index *symbols.Index, sem *semantics.Model) []string { + var names []string + for _, doc := range index.WorkspaceDocuments() { + for _, sym := range DeclaredDocumentDefinitions(index, sem, index.DocumentRoot(doc)) { + names = append(names, symbols.FQNOf(sym)) + } + } + sort.Strings(names) + return names +} + +// DocumentFiles plans the file each named document is written to when the +// documents are rendered as a set in the form of extension: its stem cut to +// fit, and tagged with a hash of the whole where two would meet letter case +// aside. Every renderer links documents to one another by this plan, so a +// document rendered on its own links to the files a set writes. Two documents +// of one name cannot be told apart. +func DocumentFiles(names []string, extension string) (map[string]string, error) { + files, err := filename.Plan(names, func(name string, tagged bool) string { + return filename.Fit(docrender.DocumentFileStem(name), extension, '.', tagged) + }) + var collision *filename.CollisionError + if errors.As(err, &collision) { + if collision.Names[0] == collision.Names[1] { + return nil, fmt.Errorf("%s names more than one document; rename one so the name is unambiguous", source.QualifiedNameText(collision.Names[0])) + } + return nil, fmt.Errorf("%s and %s render to one file name %s; rename one so both files can coexist", source.QualifiedNameText(collision.Names[0]), source.QualifiedNameText(collision.Names[1]), collision.File) + } + return files, err +} + // SiblingDocumentPlans compiles the document definitions the workspace // documents declare other than entry, skipping any whose plan does not // compile, so a single-document render can emit the anchors incoming