(PUP-8482) Add Puppet Extended S-Expression Notation #6673

thallgren · 2018-02-22T22:10:07Z

This PR adds the Puppet Extended S-Expression Notation (PN) to Puppet and makes it available to puppet parser dump and puppet epp dump commands though the option --format <old|pn|json>.

This commit adds the Puppet Extended S-Expression Notation (PN) model to Puppet.

This commit adds a parser that can parse a string in PN format into a PN model.

This commit adds a transformer capable of transforming a puppet AST into a PN model. Only a limited set of unit tests are provided because the full set (70+ transformations) is expected to be tested by a pspec test (to be provided in a separate ticket regarding pspec evaluator).

This commit adds the ability to create formatted output from a PN tree by passing an `PN::Indent` instance to the `#format` method of the elements.

This commit adds the ability to generated PN output in native or JSON form from the commands `puppet parser dump` and `puppet epp dump`. Two opions are added: --format <old|pn|json> --pretty The 'old' format is the default and outputs using the old dump format. 'pn' will output the PN tree using it's native representation. 'json' will output the PN tree using JSON representation.

jhelwig · 2018-02-22T22:24:24Z

lib/puppet/pops/parser/pn_parser.rb

+  end
+
+  # All methods below belong to the PN lexer
+  def self.char_types


If the class method is meant to be private, you'll want to mark it private using private_class_method :char_types after the method definition, since def self.foo methods aren't affected by being after private.

Good point, there's no reason for char_types be public.

Hmm. Turns out it cannot be private. An instance is not allowed to call a private method on the class.

I was mainly going off of guessing at the intent, since it appears in the file after private on line 51.

Yeah. It's a trade-off I guess. I want everything belonging to the lexer to be grouped below the comment at line 130 but it's cumbersome to mark each an every instance method private.

puppetcla · 2018-02-23T00:00:24Z

CLA signed by all contributors.

hlindberg · 2018-02-26T14:20:16Z

lib/puppet/face/epp.rb

+      'pn' is the Puppet Extended S-Expression Notation.
+      'json' outputs the same graph as 'pn' but with JSON syntax.
+
+      The output will be "pretty printed" when the option --pretty is given together with --format 'pn' or'json'. 


missing space between or and ´'json'`

hlindberg · 2018-02-26T14:21:21Z

lib/puppet/face/epp.rb

+      The output format can be controlled using the --format <old|pn|json>
+      'old' is the default, but now deprecated format which is not API.
+      'pn' is the Puppet Extended S-Expression Notation.
+      'json' outputs the same graph as 'pn' but with JSON syntax.


Maybe line 113 should end with where: and each of the three formats starts with a * bullet ?

hlindberg · 2018-02-26T14:23:08Z

lib/puppet/face/parser.rb

+      The output format can be controlled using the --format <old|pn|json>
+      'old' is the default, but now deprecated format which is not API.
+      'pn' is the Puppet Extended S-Expression Notation.
+      'json' outputs the same graph as 'pn' but with JSON syntax.


Same as earlier comment for the explanations...

hlindberg · 2018-02-26T14:23:26Z

lib/puppet/face/parser.rb

+      'pn' is the Puppet Extended S-Expression Notation.
+      'json' outputs the same graph as 'pn' but with JSON syntax.
+
+      The output will be "pretty printed" when the option --pretty is given together with --format 'pn' or'json'. 


Same as earlier comment for or'json'

hlindberg · 2018-02-26T14:36:24Z

lib/puppet/pops/pn.rb

+end
+
+require_relative 'model/pn_transformer'
+require_relative 'parser/pn_parser'


missing newline on last line

hlindberg · 2018-02-26T14:37:20Z

spec/unit/face/epp_face_spec.rb

@@ -174,6 +174,59 @@
      expect(@logs[0].message).to match(/Syntax error at 'b'/)
      expect(@logs[0].level).to eq(:err)
    end
+
+    context "using 'pp' format" do


'pn' format? instead of 'pp' ?

hlindberg · 2018-02-26T14:38:02Z

spec/unit/face/epp_face_spec.rb

@@ -174,6 +174,59 @@
      expect(@logs[0].message).to match(/Syntax error at 'b'/)
      expect(@logs[0].level).to eq(:err)
    end
+
+    context "using 'pp' format" do
+      it "prints the AST of the passed expression in PN format" do


"passed expression" => "given expression"

hlindberg · 2018-02-26T14:41:02Z

spec/unit/face/parser_spec.rb

@@ -145,6 +145,84 @@
        parser.dump(manifest)
      }.to_not raise_error
    end
+
+    context "using 'pn' format" do
+      it "prints the AST of the passed expression in PN format" do


"passed" => "given"

hlindberg · 2018-02-26T14:41:32Z

spec/unit/face/parser_spec.rb

+    end
+
+    context "using 'json' format" do
+      it "prints the AST of the passed expression in JSON based on the PN format" do


passed / given

- Insert missing space before 'json' in dump docs for epp and parser - Use * bullet in docs in dump docs for epp and parser - Add missing newline - Fix typo 'pp' should be 'pn' - Use word 'given' instead of 'passed' in several test titles

thallgren added 5 commits February 22, 2018 15:24

(PUP-8482) Add PN model

cb616d9

This commit adds the Puppet Extended S-Expression Notation (PN) model to Puppet.

(PUP-8482) Add a parser for the PN string format

7aa5ae7

This commit adds a parser that can parse a string in PN format into a PN model.

(PUP-8482) Add "pretty printing" of PN tree

c0de913

This commit adds the ability to create formatted output from a PN tree by passing an `PN::Indent` instance to the `#format` method of the elements.

thallgren requested a review from hlindberg February 22, 2018 22:10

jhelwig reviewed Feb 22, 2018

View reviewed changes

(PUP-8482) Remove unused variable causing rubocop failure

0608a76

hlindberg suggested changes Feb 26, 2018

View reviewed changes

hlindberg mentioned this pull request Feb 26, 2018

DO NOT MERGE. PN implementation. For review only #6652

Closed

hlindberg added the Language label Feb 26, 2018

(PUP-8482) Edits in respond to code review

fb9f8e6

- Insert missing space before 'json' in dump docs for epp and parser - Use * bullet in docs in dump docs for epp and parser - Add missing newline - Fix typo 'pp' should be 'pn' - Use word 'given' instead of 'passed' in several test titles

thallgren force-pushed the issue/pup-8482/pn-representation branch from d388b39 to fb9f8e6 Compare March 1, 2018 22:29

hlindberg approved these changes Mar 2, 2018

View reviewed changes

hlindberg merged commit 9441bc8 into puppetlabs:master Mar 2, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(PUP-8482) Add Puppet Extended S-Expression Notation #6673

(PUP-8482) Add Puppet Extended S-Expression Notation #6673

thallgren commented Feb 22, 2018

jhelwig Feb 22, 2018

thallgren Feb 22, 2018

thallgren Feb 22, 2018

jhelwig Feb 22, 2018

thallgren Feb 22, 2018

puppetcla commented Feb 23, 2018

hlindberg Feb 26, 2018

hlindberg Feb 26, 2018

hlindberg Feb 26, 2018

hlindberg Feb 26, 2018

hlindberg Feb 26, 2018

hlindberg Feb 26, 2018

hlindberg Feb 26, 2018

hlindberg Feb 26, 2018

hlindberg Feb 26, 2018

(PUP-8482) Add Puppet Extended S-Expression Notation #6673

(PUP-8482) Add Puppet Extended S-Expression Notation #6673

Conversation

thallgren commented Feb 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

puppetcla commented Feb 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment