refactor(parser): simplify substitution processing (#910)

simplify logic to process substitutions on elements also: - rename `AttributeSubstitution` to `AttributeReference` in grammar rules and in type in `pkg/types` - use single `string` for `Location.Path` when there's no attribute reference instead of `[]interface{}{*types.StringElement{Content:...}}` - wrap roles and options attribute values in `types.Roles` and `types.Options` respectivily (hence avoid matching `[]interface{}`) - change logic to handle `\n` at the end of paragraph lines - remove outdated grammar rules Signed-off-by: Xavier Coulon <[email protected]>
bytesparadise · Jan 29, 2022 · db29560 · db29560
1 parent 771933b
commit db29560
Show file tree

Hide file tree

Showing 82 changed files with 30,495 additions and 56,219 deletions.
diff --git a/libasciidoc_test.go b/libasciidoc_test.go
@@ -316,7 +316,7 @@ Last updated {{.LastUpdated}}
 		Context("with body only", func() {
 
 			It("should render valid manpage", func() {
-				logs, reset := ConfigureLogger(log.WarnLevel)
+				_, reset := ConfigureLogger(log.WarnLevel)
 				defer reset()
 				source := `= eve(1)
 Andrew Stanton
@@ -366,14 +366,13 @@ Free use of this software is granted under the terms of the MIT License.</p>
 					))
 				Expect(err).NotTo(HaveOccurred())
 				Expect(out.String()).To(MatchHTML(expectedContent))
-				Expect(logs).To(ContainJSONLog(log.WarnLevel, "unable to find attribute 'author'"))
 			})
 		})
 
 		Context("full", func() {
 
 			It("should render valid manpage", func() {
-				logs, reset := ConfigureLogger(log.WarnLevel)
+				_, reset := ConfigureLogger(log.WarnLevel)
 				defer reset()
 				source := `= eve(1)
 Andrew Stanton
@@ -451,7 +450,6 @@ Last updated {{.LastUpdated}}
 					))
 				Expect(err).NotTo(HaveOccurred())
 				Expect(out.String()).To(MatchHTMLTemplate(expectedContent, lastUpdated))
-				Expect(logs).To(ContainJSONLog(log.WarnLevel, "unable to find attribute 'author'"))
 			})
 
 			It("should render invalid manpage as article", func() {
@@ -541,7 +539,6 @@ Last updated {{.LastUpdated}}
 					))
 				Expect(err).NotTo(HaveOccurred())
 				Expect(out.String()).To(MatchHTMLTemplate(expectedContent, lastUpdated))
-				Expect(logs).To(ContainJSONLog(log.WarnLevel, "unable to find attribute 'author'"))
 				Expect(logs).To(ContainJSONLog(log.WarnLevel, "changing doctype to 'article' because problems were found in the document"))
 				Expect(logs).To(ContainJSONLog(log.ErrorLevel, "manpage document is missing the 'Name' section"))
 			})

diff --git a/pkg/parser/attributes_test.go b/pkg/parser/attributes_test.go
@@ -29,9 +29,7 @@ var _ = Describe("attributes", func() {
 					Elements: []interface{}{
 						&types.ImageBlock{
 							Location: &types.Location{
-								Path: []interface{}{
-									&types.StringElement{Content: "foo.png"},
-								},
+								Path: "foo.png",
 							},
 						},
 					},
@@ -51,9 +49,7 @@ var _ = Describe("attributes", func() {
 					Elements: []interface{}{
 						&types.ImageBlock{
 							Location: &types.Location{
-								Path: []interface{}{
-									&types.StringElement{Content: "foo.png"},
-								},
+								Path: "foo.png",
 							},
 						},
 					},
@@ -73,9 +69,7 @@ var _ = Describe("attributes", func() {
 					Elements: []interface{}{
 						&types.ImageBlock{
 							Location: &types.Location{
-								Path: []interface{}{
-									&types.StringElement{Content: "foo.png"},
-								},
+								Path: "foo.png",
 							},
 						},
 					},
@@ -98,9 +92,7 @@ var _ = Describe("attributes", func() {
 								types.AttrWidth: "200",
 							},
 							Location: &types.Location{
-								Path: []interface{}{
-									&types.StringElement{Content: "foo.png"},
-								},
+								Path: "foo.png",
 							},
 						},
 					},
@@ -123,9 +115,7 @@ var _ = Describe("attributes", func() {
 								types.AttrImageAlt: `Quoted, Here`,
 							},
 							Location: &types.Location{
-								Path: []interface{}{
-									&types.StringElement{Content: "foo.png"},
-								},
+								Path: "foo.png",
 							},
 						},
 					},
@@ -148,9 +138,7 @@ var _ = Describe("attributes", func() {
 								types.AttrImageAlt: `The Foo"Bar" here`,
 							},
 							Location: &types.Location{
-								Path: []interface{}{
-									&types.StringElement{Content: "foo.png"},
-								},
+								Path: "foo.png",
 							},
 						},
 					},
@@ -173,9 +161,7 @@ var _ = Describe("attributes", func() {
 								types.AttrImageAlt: `The Foo'Bar' here`,
 							},
 							Location: &types.Location{
-								Path: []interface{}{
-									&types.StringElement{Content: "foo.png"},
-								},
+								Path: "foo.png",
 							},
 						},
 					},
@@ -198,9 +184,7 @@ var _ = Describe("attributes", func() {
 								types.AttrImageAlt: `The Foo\Bar here`,
 							},
 							Location: &types.Location{
-								Path: []interface{}{
-									&types.StringElement{Content: "foo.png"},
-								},
+								Path: "foo.png",
 							},
 						},
 					},
@@ -223,9 +207,7 @@ var _ = Describe("attributes", func() {
 								types.AttrImageAlt: `The Foo\Bar here`,
 							},
 							Location: &types.Location{
-								Path: []interface{}{
-									&types.StringElement{Content: "foo.png"},
-								},
+								Path: "foo.png",
 							},
 						},
 					},
@@ -249,9 +231,7 @@ var _ = Describe("attributes", func() {
 								types.AttrHeight:   "100",
 							},
 							Location: &types.Location{
-								Path: []interface{}{
-									&types.StringElement{Content: "foo.png"},
-								},
+								Path: "foo.png",
 							},
 						},
 					},
@@ -278,9 +258,7 @@ var _ = Describe("attributes", func() {
 								types.AttrWidth:    "1",
 							},
 							Location: &types.Location{
-								Path: []interface{}{
-									&types.StringElement{Content: "foo.png"},
-								},
+								Path: "foo.png",
 							},
 						},
 					},
@@ -307,9 +285,7 @@ var _ = Describe("attributes", func() {
 								"test2":            "second test", // shows trailing pad removed
 							},
 							Location: &types.Location{
-								Path: []interface{}{
-									&types.StringElement{Content: "foo.png"},
-								},
+								Path: "foo.png",
 							},
 						},
 					},
@@ -336,9 +312,7 @@ var _ = Describe("attributes", func() {
 								"test2":            `second "test"`, // shows trailing pad removed
 							},
 							Location: &types.Location{
-								Path: []interface{}{
-									&types.StringElement{Content: "foo.png"},
-								},
+								Path: "foo.png",
 							},
 						},
 					},
@@ -555,19 +529,19 @@ var _ = DescribeTable("valid block attributes",
 	// role shorthand
 	Entry(`[.a_role]`, `[.a_role]`,
 		types.Attributes{
-			types.AttrRoles: []interface{}{`a_role`},
+			types.AttrRoles: types.Roles{`a_role`},
 		},
 	),
 	Entry(`[.a_role.another_role]`, `[.a_role.another_role]`,
 		types.Attributes{
-			types.AttrRoles: []interface{}{`a_role`, `another_role`},
+			types.AttrRoles: types.Roles{`a_role`, `another_role`},
 		},
 	),
 	Entry(`[source.a_role,go]`, `[source.a_role,go]`,
 		types.Attributes{
 			types.AttrPositional1: `source`,
 			types.AttrPositional2: `go`,
-			types.AttrRoles:       []interface{}{`a_role`},
+			types.AttrRoles:       types.Roles{`a_role`},
 		},
 	),
 	Entry(`[source,go.not_a_role]`, `[source,go.not_a_role]`,
@@ -580,34 +554,34 @@ var _ = DescribeTable("valid block attributes",
 	// options (explicit)
 	Entry(`[options=header]`, `[options=header]`,
 		types.Attributes{
-			types.AttrOptions: []interface{}{"header"},
+			types.AttrOptions: types.Options{"header"},
 		},
 	),
 	Entry(`[options="header,footer"]`, `[options="header,footer"]`,
 		types.Attributes{
-			types.AttrOptions: []interface{}{"header", "footer"},
+			types.AttrOptions: types.Options{"header", "footer"},
 		},
 	),
 
 	// option shorthand
 	Entry(`[%hardbreaks]`, `[%hardbreaks]`,
 		types.Attributes{
-			types.AttrOptions: []interface{}{"hardbreaks"},
+			types.AttrOptions: types.Options{"hardbreaks"},
 		},
 	),
 	Entry(`[%header]`, `[%header]`,
 		types.Attributes{
-			types.AttrOptions: []interface{}{"header"},
+			types.AttrOptions: types.Options{"header"},
 		},
 	),
 	Entry(`[%footer]`, `[%footer]`,
 		types.Attributes{
-			types.AttrOptions: []interface{}{"footer"},
+			types.AttrOptions: types.Options{"footer"},
 		},
 	),
 	Entry(`[%header%footer]`, `[%header%footer]`,
 		types.Attributes{
-			types.AttrOptions: []interface{}{"header", "footer"},
+			types.AttrOptions: types.Options{"header", "footer"},
 		},
 	),
 
@@ -621,27 +595,27 @@ var _ = DescribeTable("valid block attributes",
 	Entry(`[#an_id.a_role]`, `[#an_id.a_role]`,
 		types.Attributes{
 			types.AttrID:    `an_id`,
-			types.AttrRoles: []interface{}{`a_role`},
+			types.AttrRoles: types.Roles{`a_role`},
 		},
 	),
 	Entry(`[#an_id.role_1%option_1.role_2]`, `[#an_id.role_1%option_1.role_2]`,
 		types.Attributes{
 			types.AttrID:      `an_id`,
-			types.AttrRoles:   []interface{}{`role_1`, `role_2`},
-			types.AttrOptions: []interface{}{`option_1`},
+			types.AttrRoles:   types.Roles{`role_1`, `role_2`},
+			types.AttrOptions: types.Options{`option_1`},
 		},
 	),
 	Entry(`[#an_id.role_1%option_1.role_2%option_2]`, `[#an_id.role_1%option_1.role_2%option_2]`,
 		types.Attributes{
 			types.AttrID:      `an_id`,
-			types.AttrRoles:   []interface{}{`role_1`, `role_2`},
-			types.AttrOptions: []interface{}{`option_1`, `option_2`},
+			types.AttrRoles:   types.Roles{`role_1`, `role_2`},
+			types.AttrOptions: types.Options{`option_1`, `option_2`},
 		},
 	),
 	Entry(`[#an_id,role="a role"]`, `[#an_id,role="a role"]`,
 		types.Attributes{
 			types.AttrID:    `an_id`,
-			types.AttrRoles: []interface{}{`a role`},
+			types.AttrRoles: types.Roles{`a role`},
 		},
 	),
 	Entry(`[qanda#quiz]`, `[qanda#quiz]`,

diff --git a/pkg/parser/check_list_test.go b/pkg/parser/check_list_test.go
@@ -105,7 +105,7 @@ var _ = Describe("checked lists", func() {
 					&types.List{
 						Kind: types.UnorderedListKind,
 						Attributes: types.Attributes{
-							types.AttrOptions: []interface{}{
+							types.AttrOptions: types.Options{
 								types.AttrInteractive,
 							},
 						},

diff --git a/pkg/parser/comment_test.go b/pkg/parser/comment_test.go
@@ -85,10 +85,10 @@ another line`
 						&types.Paragraph{
 							Elements: []interface{}{
 								&types.StringElement{
-									Content: "a first line",
+									Content: "a first line\n",
 								},
 								&types.StringElement{
-									Content: "\nanother line",
+									Content: "another line",
 								},
 							},
 						},

diff --git a/pkg/parser/context.go b/pkg/parser/context.go
@@ -11,7 +11,7 @@ import (
 
 type ParseContext struct {
 	filename     string
-	Opts         []Option
+	Opts         []Option // TODO: unexport this field?
 	levelOffsets levelOffsets
 	attributes   *contextAttributes
 	userMacros   map[string]configuration.MacroTemplate
@@ -25,7 +25,7 @@ func NewParseContext(config *configuration.Configuration, opts ...Option) *Parse
 	opts = append(opts, Entrypoint("DocumentFragment"))
 	opts = append(opts, GlobalStore(frontMatterKey, true))
 	opts = append(opts, GlobalStore(documentHeaderKey, true))
-	opts = append(opts, GlobalStore(substitutionKey, newNormalSubstitution()))
+	opts = append(opts, GlobalStore(enabledSubstitutions, []string{Attributes}))
 	opts = append(opts, GlobalStore(usermacrosKey, config.Macros))
 	return &ParseContext{
 		filename:     config.Filename,
@@ -38,19 +38,24 @@ func NewParseContext(config *configuration.Configuration, opts ...Option) *Parse
 }
 
 func (c *ParseContext) Clone() *ParseContext {
-	opts := make([]Option, len(c.Opts))
-	copy(opts, c.Opts)
-	attributes := c.attributes.clone()
 	return &ParseContext{
 		filename:     c.filename,
-		Opts:         opts,
+		Opts:         options(c.Opts).clone(),
 		levelOffsets: c.levelOffsets.clone(),
-		attributes:   attributes,
+		attributes:   c.attributes.clone(),
 		userMacros:   c.userMacros,
 		counters:     c.counters,
 	}
 }
 
+type options []Option
+
+func (o options) clone() []Option {
+	result := make([]Option, len(o))
+	copy(result, o)
+	return result
+}
+
 type contextAttributes struct {
 	immutableAttributes types.Attributes
 	attributes          types.Attributes
@@ -84,14 +89,6 @@ func (a *contextAttributes) has(k string) bool {
 	return found
 }
 
-func (a *contextAttributes) get(k string) interface{} {
-	value, found := a.immutableAttributes[k]
-	if found {
-		return value
-	}
-	return a.attributes[k]
-}
-
 func (a *contextAttributes) allAttributes() map[string]interface{} {
 	result := make(map[string]interface{}, len(a.attributes)+len(a.immutableAttributes))
 	for k, v := range a.attributes {
@@ -111,13 +108,6 @@ func (a *contextAttributes) getAsString(k string) (string, bool, error) {
 	return a.attributes.GetAsString(k)
 }
 
-func (a *contextAttributes) getAsStringWithDefault(k string, defaultValue string) string {
-	if a.immutableAttributes.Has(k) {
-		return a.immutableAttributes.GetAsStringWithDefault(k, defaultValue)
-	}
-	return a.attributes.GetAsStringWithDefault(k, defaultValue)
-}
-
 func (a *contextAttributes) getAsIntWithDefault(k string, defaultValue int) int {
 	if a.immutableAttributes.Has(k) {
 		return a.immutableAttributes.GetAsIntWithDefault(k, defaultValue)