Add single-line pragma

Summary: We have a number of partials that render fragments of lines, for example the template argument of the return type of a method. In order to keep the output readable we suppress newlines in these partials by commenting them out, which adds O(n) syntactic noise. This hurts readability and adds overhead to modifying these partials. Handlebars supports [whitespace control](https://handlebarsjs.com/guide/expressions.html#whitespace-control) for statements, which is less disruptive than our comment-based approach but still introduces O(n) syntactic noise. Instead we introduce an O(1) syntax for suppressing all newlines in the current partial. To prevent confusing action-at-a-distance it is not applied transitively to included partials, and the included partial is also expected to have this pragma already. Reviewed By: yoney Differential Revision: D67611032 fbshipit-source-id: d8486196a98763483e3a47e5774f0ea8b0d04c1b
facebook · Dec 24, 2024 · b26e85a · b26e85a
1 parent 4690549
commit b26e85a
Show file tree

Hide file tree

Showing 11 changed files with 212 additions and 16 deletions.
diff --git a/.../thrift/src/thrift/compiler/generate/templates/cpp2/module_types_tcc/struct_type.mustache b/.../thrift/src/thrift/compiler/generate/templates/cpp2/module_types_tcc/struct_type.mustache
@@ -14,18 +14,19 @@
   See the License for the specific language governing permissions and
   limitations under the License.
 
-}}{{#type:void?}}T_VOID{{/type:void?}}{{!
-}}{{#type:string_or_binary?}}T_STRING{{/type:string_or_binary?}}{{!
-}}{{#type:bool?}}T_BOOL{{/type:bool?}}{{!
-}}{{#type:byte?}}T_BYTE{{/type:byte?}}{{!
-}}{{#type:i16?}}T_I16{{/type:i16?}}{{!
-}}{{#type:i32?}}T_I32{{/type:i32?}}{{!
-}}{{#type:i64?}}T_I64{{/type:i64?}}{{!
-}}{{#type:double?}}T_DOUBLE{{/type:double?}}{{!
-}}{{#type:float?}}T_FLOAT{{/type:float?}}{{!
-}}{{#type:enum?}}T_I32{{/type:enum?}}{{!
-}}{{#type:map?}}T_MAP{{/type:map?}}{{!
-}}{{#type:set?}}T_SET{{/type:set?}}{{!
-}}{{#type:list?}}T_LIST{{/type:list?}}{{!
-}}{{#type:struct?}}T_STRUCT{{/type:struct?}}{{!
 }}
+{{#pragma single-line}}
+{{#type:void?}}T_VOID{{/type:void?}}
+{{#type:string_or_binary?}}T_STRING{{/type:string_or_binary?}}
+{{#type:bool?}}T_BOOL{{/type:bool?}}
+{{#type:byte?}}T_BYTE{{/type:byte?}}
+{{#type:i16?}}T_I16{{/type:i16?}}
+{{#type:i32?}}T_I32{{/type:i32?}}
+{{#type:i64?}}T_I64{{/type:i64?}}
+{{#type:double?}}T_DOUBLE{{/type:double?}}
+{{#type:float?}}T_FLOAT{{/type:float?}}
+{{#type:enum?}}T_I32{{/type:enum?}}
+{{#type:map?}}T_MAP{{/type:map?}}
+{{#type:set?}}T_SET{{/type:set?}}
+{{#type:list?}}T_LIST{{/type:list?}}
+{{#type:struct?}}T_STRUCT{{/type:struct?}}
diff --git a/third-party/thrift/src/thrift/compiler/whisker/ast.cc b/third-party/thrift/src/thrift/compiler/whisker/ast.cc
@@ -86,4 +86,11 @@ std::string_view expression::function_call::name() const {
       [&](function_call::or_tag) { return "or"; });
 }
 
+std::string_view pragma_statement::to_string() const {
+  switch (pragma) {
+    case pragma_statement::pragmas::single_line:
+      return "single-line";
+  }
+}
+
 } // namespace whisker::ast
diff --git a/third-party/thrift/src/thrift/compiler/whisker/ast.h b/third-party/thrift/src/thrift/compiler/whisker/ast.h
@@ -34,6 +34,7 @@ struct with_block;
 struct partial_apply;
 struct interpolation;
 struct let_statement;
+struct pragma_statement;
 
 /**
  * The top-level types of constructs allowed in a Whisker source file.
@@ -47,6 +48,7 @@ using body = std::variant<
     conditional_block,
     with_block,
     let_statement,
+    pragma_statement,
     partial_apply>;
 using bodies = std::vector<body>;
 
@@ -227,6 +229,20 @@ struct let_statement {
   expression value;
 };
 
+/**
+ * A Whisker construct for changing rendering behavior.
+ */
+struct pragma_statement {
+  enum class pragmas {
+    single_line,
+  };
+  source_range loc;
+
+  pragmas pragma;
+
+  std::string_view to_string() const;
+};
+
 /**
  * A Whisker construct for conditionals and/or iteration. This matches Mustache:
  *   https://mustache.github.io/mustache.5.html#Sections

diff --git a/third-party/thrift/src/thrift/compiler/whisker/parser.cc b/third-party/thrift/src/thrift/compiler/whisker/parser.cc
@@ -27,6 +27,7 @@
 #include <iterator>
 #include <map>
 #include <optional>
+
 #include <utility>
 #include <variant>
 #include <vector>
@@ -757,10 +758,11 @@ class parser {
       ast::conditional_block,
       ast::with_block,
       ast::let_statement,
+      ast::pragma_statement,
       ast::partial_apply>;
   // template → { interpolation | block | statement | partial-apply }
   // block → { section-block | conditional-block }
-  // statement → { let-statement }
+  // statement → { let-statement | pragma-statement }
   parse_result<template_body> parse_template(parser_scan_window scan) {
     assert(scan.empty());
     if (scan.peek().kind != tok::open) {
@@ -789,6 +791,8 @@ class parser {
       templ = std::move(with_block).consume_and_advance(&scan);
     } else if (parse_result let_statement = parse_let_statement(scan)) {
       templ = std::move(let_statement).consume_and_advance(&scan);
+    } else if (parse_result pragma_statement = parse_pragma_statement(scan)) {
+      templ = std::move(pragma_statement).consume_and_advance(&scan);
     } else if (parse_result section_block = parse_section_block(scan)) {
       templ = std::move(section_block).consume_and_advance(&scan);
     } else if (parse_result partial_apply = parse_partial_apply(scan)) {
@@ -1085,6 +1089,45 @@ class parser {
         scan};
   }
 
+  // pragma-statement → { "{{" ~ "#" ~ "pragma" ~ identifier ~ "}}" }
+  parse_result<ast::pragma_statement> parse_pragma_statement(
+      parser_scan_window scan) {
+    assert(scan.empty());
+    const auto scan_start = scan.start;
+
+    if (!try_consume_token(&scan, tok::open)) {
+      return no_parse_result();
+    }
+    if (!try_consume_token(&scan, tok::pound)) {
+      return no_parse_result();
+    }
+    if (!try_consume_token(&scan, tok::kw_pragma)) {
+      return no_parse_result();
+    }
+
+    const token& id = scan.peek();
+    if (id.kind != tok::identifier) {
+      report_expected(scan, "identifier in pragma-statement");
+    }
+    scan.advance();
+
+    ast::pragma_statement::pragmas pragma;
+    if (id.string_value() == "single-line") {
+      pragma = ast::pragma_statement::pragmas::single_line;
+    } else {
+      report_error(scan, "unknown pragma `{}`", id.string_value());
+    }
+
+    if (!try_consume_token(&scan, tok::close)) {
+      report_expected(
+          scan, fmt::format("{} to close pragma-statement", tok::close));
+    }
+
+    return {
+        ast::pragma_statement{scan.with_start(scan_start).range(), pragma},
+        scan};
+  }
+
   // conditional-block →
   //   { cond-block-open ~ body* ~ else-block? ~ cond-block-close }
   // cond-block-open →

diff --git a/third-party/thrift/src/thrift/compiler/whisker/print_ast.cc b/third-party/thrift/src/thrift/compiler/whisker/print_ast.cc
@@ -137,6 +137,14 @@ struct ast_visitor {
     visit(let_statement.id, scope.open_property());
     visit(let_statement.value, scope.open_property());
   }
+  void visit(
+      const ast::pragma_statement& pragma_statement,
+      tree_printer::scope scope) const {
+    scope.println(
+        " pragma-statement `{}` {}",
+        pragma_statement.to_string(),
+        location(pragma_statement.loc));
+  }
   // Prevent implicit conversion to ast::body. Otherwise, we can silently
   // compile an infinitely recursive visit() chain if there is a missing
   // overload for one of the alternatives in the variant.

diff --git a/third-party/thrift/src/thrift/compiler/whisker/render.cc b/third-party/thrift/src/thrift/compiler/whisker/render.cc
@@ -23,6 +23,7 @@
 #include <exception>
 #include <functional>
 #include <ostream>
+#include <stack>
 #include <string>
 #include <vector>
 
@@ -256,7 +257,11 @@ class render_engine {
   }
 
   void visit(const ast::text& text) { out_.write(text); }
-  void visit(const ast::newline& newline) { out_.write(newline); }
+  void visit(const ast::newline& newline) {
+    if (!skip_newlines_.top()) {
+      out_.write(newline);
+    }
+  }
   void visit(const ast::comment&) {
     // comments are not rendered in the output
   }
@@ -384,6 +389,15 @@ class render_engine {
         });
   }
 
+  void visit(const ast::pragma_statement& pragma_statement) {
+    using pragma = ast::pragma_statement::pragmas;
+    switch (pragma_statement.pragma) {
+      case pragma::single_line:
+        skip_newlines_.top() = true;
+        break;
+    }
+  }
+
   void visit(const ast::interpolation& interpolation) {
     const object& value = evaluate(interpolation.content);
 
@@ -564,6 +578,29 @@ class render_engine {
     }
   }
 
+  /**
+   * An RAII guard that disables printing newlines. This supports the
+   * single-line pragma for partials.
+   */
+  auto make_single_line_guard(bool enabled) {
+    class single_line_guard {
+     public:
+      explicit single_line_guard(render_engine& engine, bool skip)
+          : engine_(engine) {
+        engine_.skip_newlines_.push(skip);
+      }
+      ~single_line_guard() { engine_.skip_newlines_.pop(); }
+      single_line_guard(single_line_guard&& other) = delete;
+      single_line_guard& operator=(single_line_guard&& other) = delete;
+      single_line_guard(const single_line_guard& other) = delete;
+      single_line_guard& operator=(const single_line_guard& other) = delete;
+
+     private:
+      render_engine& engine_;
+    };
+    return single_line_guard{*this, enabled};
+  }
+
   void visit(const ast::with_block& with_block) {
     const ast::expression& expr = with_block.value;
     object value = evaluate(expr);
@@ -625,13 +662,15 @@ class render_engine {
     // execute within the scope where they are invoked.
     auto indent_guard =
         out_.make_indent_guard(partial_apply.standalone_offset_within_line);
+    auto single_line_guard = make_single_line_guard(false);
     visit(resolved_partial->body_elements);
   }
 
   outputter out_;
   eval_context eval_context_;
   diagnostics_engine& diags_;
   render_options opts_;
+  std::stack<bool> skip_newlines_{{false}};
 };
 
 } // namespace

diff --git a/third-party/thrift/src/thrift/compiler/whisker/test/parser_test.cc b/third-party/thrift/src/thrift/compiler/whisker/test/parser_test.cc
@@ -704,6 +704,26 @@ TEST_F(ParserTest, let_statement_keyword) {
           1)));
 }
 
+TEST_F(ParserTest, pragma_single_line) {
+  auto ast = parse_ast("{{#pragma single-line}}\n");
+  EXPECT_THAT(diagnostics, testing::IsEmpty());
+  EXPECT_EQ(
+      to_string(*ast),
+      "root [path/to/test-1.whisker]\n"
+      "|- pragma-statement `single-line` <line:1:1, col:24>\n");
+}
+
+TEST_F(ParserTest, pragma_unrecognized) {
+  auto ast = parse_ast("{{#pragma unrecognized}}\n");
+  EXPECT_THAT(
+      diagnostics,
+      testing::ElementsAre(diagnostic(
+          diagnostic_level::error,
+          "unknown pragma `unrecognized`",
+          path_to_file(1),
+          1)));
+}
+
 TEST_F(ParserTest, with_block) {
   auto ast = parse_ast(
       "{{#with foo.bar}}\n"

diff --git a/third-party/thrift/src/thrift/compiler/whisker/test/render_test.cc b/third-party/thrift/src/thrift/compiler/whisker/test/render_test.cc
@@ -1313,4 +1313,32 @@ TEST_F(RenderTest, globals_shadowing) {
   EXPECT_EQ(*result, "good\n");
 }
 
+TEST_F(RenderTest, pragma_single_line) {
+  auto result = render(
+      "{{#pragma single-line}}\n"
+      "This\n"
+      " is\n"
+      "{{! comment}}\n"
+      " all\n"
+      " one\n"
+      " line\n",
+      w::map({}));
+  EXPECT_THAT(diagnostics(), testing::IsEmpty());
+  EXPECT_EQ(*result, "This is all one line");
+
+  // Test that the pragma is scoped to the active partial/template.
+  result = render(
+      "{{#pragma single-line}}\n"
+      "This\n"
+      " is\n"
+      "{{> partial }}\n"
+      " all\n"
+      " one\n"
+      " line\n",
+      w::map({}),
+      partials({{"partial", "\nnot\n!\n"}}));
+  EXPECT_THAT(diagnostics(), testing::IsEmpty());
+  EXPECT_EQ(*result, "This is\nnot\n!\n all one line");
+}
+
 } // namespace whisker
diff --git a/third-party/thrift/src/thrift/compiler/whisker/token.cc b/third-party/thrift/src/thrift/compiler/whisker/token.cc
@@ -45,6 +45,7 @@
   WHISKER_KEYWORD(and)     \
   WHISKER_KEYWORD(or)      \
   WHISKER_KEYWORD(not)     \
+  WHISKER_KEYWORD(pragma)  \
   WHISKER_KEYWORD(with)    \
   WHISKER_KEYWORD(this)    \
   WHISKER_KEYWORD(define)  \

diff --git a/third-party/thrift/src/thrift/compiler/whisker/token.h b/third-party/thrift/src/thrift/compiler/whisker/token.h
@@ -82,6 +82,7 @@ enum class tok : unsigned {
   kw_and,
   kw_or,
   kw_not,
+  kw_pragma,
 
   // Reserved Words (may be used in the future):
   kw_with,

diff --git a/third-party/thrift/src/thrift/doc/contributions/whisker.md b/third-party/thrift/src/thrift/doc/contributions/whisker.md
@@ -594,6 +594,38 @@ let-statement → { "{{" ~ "#" ~ "let" ~ identifier ~ "=" ~ expression ~ "}}" }
 
 </Grammar>
 
+### Pragma Statements
+
+Whisker `{{#pragma}}` statements allow modifying rendering behavior for the current template or partial.
+
+Currently the only pragma supported is `single-line`, which suppresses all newlines in the current partial.
+
+<Example>
+
+```handlebars
+{{#pragma single-line}}
+This
+ is
+ all
+ one
+ line
+.
+```
+
+```text title=Output
+This is all one line.
+```
+
+</Example>
+
+<Grammar>
+
+```
+pragma-statement → { "{{" ~ "#" ~ "pragma" ~ ( "single-line" ) ~ "}}" }
+```
+
+</Grammar>
+
 ### Partial Blocks
 
 :::warning
-Original file line number
+Diff line change
@@ Expand Up / @@ -82,6 +82,7 @@ enum class tok : unsigned { @@
       kw_and,
       kw_or,
       kw_not,
+      kw_pragma,
       // Reserved Words (may be used in the future):
       kw_with,
@@ Expand Down @@