Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Extra line with condition statement #1253

Open
Pierre-Monier opened this issue Feb 14, 2024 · 5 comments
Open

fix: Extra line with condition statement #1253

Pierre-Monier opened this issue Feb 14, 2024 · 5 comments
Labels
bug Something isn't working customer:🦄

Comments

@Pierre-Monier
Copy link

Pierre-Monier commented Feb 14, 2024

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

@Pierre-Monier Pierre-Monier added the bug Something isn't working label Feb 14, 2024
@alestiago
Copy link
Collaborator

alestiago commented Feb 14, 2024

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
Copy link
Author

Pierre-Monier commented Feb 14, 2024

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
Copy link
Collaborator

alestiago commented Feb 15, 2024

@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
Copy link
Author

@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
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working customer:🦄
Projects
None yet
Development

No branches or pull requests

2 participants