felangel / mason

Tools which allow developers to create and consume reusable templates called bricks.
https://docs.brickhub.dev
990 stars 99 forks source link

fix: Extra line with condition statement #1253

Open Pierre-Monier opened 9 months ago

Pierre-Monier commented 9 months ago

Description According to the documentation, this :

{{^publish}}
publish_to: none
{{/publish}}
dependencies:
  flutter:
    sdk: flutter
  {{#useGoogleFonts}}
  google_fonts: latest
  {{/useGoogleFonts}}

with the following configuration :

{
  "publish": false,
  "useGoogleFonts": true
}

should generate this :

publish_to: none
dependencies:
  flutter:
    sdk: flutter
  google_fonts: latest

But in reality, what it generates is the following :


publish_to: none

dependencies:
  flutter:
    sdk: flutter

  google_fonts: latest

As you can see, there is some extra space.

Steps To Reproduce

  1. Clone this repository
  2. at the root, run mason get && mason make demo -c config.json -o ./output
  3. See the generated file in ./output/HELLO.md

Expected Behavior

The generated file should have no extra space, as described in the doc

I use version 0.1.0-dev.53

alestiago commented 9 months ago

Hi @Pierre-Monier !

You can already solve this by not including the new line:

{{^publish}}publish_to: none{{/publish}}

dependencies:
  flutter:
    sdk: flutter{{#useGoogleFonts}}
  google_fonts: latest{{/useGoogleFonts}}

Alternatively, you can use Mustache comments to escape the line break:

{{^publish}}publish_to: none{{/publish}}

dependencies:
  flutter:
    sdk: flutter
  {{#useGoogleFonts}}{{!
}}  google_fonts: latest{{!
}}{{/useGoogleFonts}}

In my honest opinion, I think both solutions hinder readability and I would like an option that doesn't hinder the readability as much. Although, a solution that does so may start to deviate from the Mustache specification. I wonder about @felangel thoughts on this.

Pierre-Monier commented 9 months ago

Thanks for coming back !

Yes, your workaround works, but I'm in a very specific case where I will be very happy that it works as explain in the documentation :).

Also, according to the mustache specification, it should work as I describe

Screenshot 2024-02-14 at 17 31 28

You can see that

Hello Chris
You have just won 10000 dollars!
Well, 6000.0 dollars, after taxes.

is not the same as :

Hello Chris
You have just won 10000 dollars!

Well, 6000.0 dollars, after taxes.

Maybe it is an issue with mustache directly ? In that case, I can move my issue here, but I would be very surprised if it is the case

alestiago commented 9 months ago

@Pierre-Monier I agree with your suggestion considering the specification example.

{{#my_var}}
hello
{{/my_var}}

Should evaluate to:

hello

If a new line is desired one would:

{{#my_var}}

hello
{{/my_var}}

This is obviously a breaking change, however, I think we should go forward with it if the specification details so.

Pierre-Monier commented 9 months ago

@alestiago Sounds great!

Since this is a breaking change, it might be a good occasion to change the conditional syntax, like describe in this issue. I think this new syntax will be a huge improvement.

alestiago commented 9 months ago

@felangel any thoughts? How should we move forward here 🙌 👀