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

[WIP] Bugfix: Fix indentation for chained method #88

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

zackad
Copy link
Owner

@zackad zackad commented Nov 28, 2024

Close GH-87

TODO

  • Add documentation about experimental feature
  • Disable experimental feature by default

Input

<a class="task-edit"
   href="{{
   ea_url().setDashboard(
       'App\\Controller\\Dashboard\\DashboardController'
   ).setController(
       'App\\Controller\\Dashboard\\CRUD\\TacheCrudController'
   ).setAction(
       'edit'
   ).setEntityId(
       tache.id
   ).set(
       'sort',
       null
   )
   }}">
    <i class="fa-solid fa-edit"></i>
</a>

Expected Output

<a class="task-edit"
    href="{{
        ea_url()
            .setDashboard('App\\Controller\\Dashboard\\DashboardController')
            .setController('App\\Controller\\Dashboard\\CRUD\\TacheCrudController')
            .setAction('edit')
            .setEntityId(tache.id)
            .set('sort',null)
    }}">
    <i class="fa-solid fa-edit"></i>
</a>

Current Implementation Output

<a class="task-edit"
    href="{{
    ea_url().setDashboard('App\\Controller\\Dashboard\\DashboardController')
        .setController('App\\Controller\\Dashboard\\CRUD\\TacheCrudController')
        .setAction('edit')
        .setEntityId(tache.id)
        .set('sort',null)
    }}">
    <i class="fa-solid fa-edit"></i>
</a>

@rellafella
Copy link
Collaborator

Another couple of examples from the Craft CMS universe if this is useful.

{% do seomatic.meta
    .seoTitle("Page title")
    .seoDescription("Page description")
    .siteNamePosition("none")
    .ogSiteNamePosition("none")
    .ogType("website")
    .ogTitle("Page title")
    .ogDescription("Page description")
    .ogImage("https://example.com/image.jpg")
    .ogImageDescription("Image description")
    .twitterCard("summary")
    .twitterCreator("@exampleHandle")
    .twitterTitle("Page title")
    .twitterDescription("Page description")
    .twitterImageDescription("Image description")
    .twitterSiteNamePosition("none") %}

Should probably be noted here that entries is valid as a method or a property so could either be entries()

{% set posts = craft.entries
    .section("news")
    .orderBy("postDate DESC")
    .limit(10)
    .all() %}

@zackad
Copy link
Owner Author

zackad commented Dec 3, 2024

{% set posts = craft.entries
    .section("news")
    .orderBy("postDate DESC")
    .limit(10)
    .all() %}

I'm surprised that people actually writing code like this. It defeat the purpose of templating engine. Putting business logic inside presentation layer is bad idea, IMO.

Personal preference aside, this example is related to #91. If we print expression opening/closing in the same line, it won't be obvious where the start/end of expression since they're on different level of indentation.

Current behaviour

{{
    var.method()
        .otherMetod()
}}

If bracket on same line

{{ var.method()
    .otherMetod() }}

This output is not so "pretty" IMO.

@rellafella
Copy link
Collaborator

rellafella commented Dec 3, 2024

I'm surprised that people actually writing code like this. It defeat the purpose of templating engine. Putting business logic inside presentation layer is bad idea, IMO.

I think that's just the nature of having templating baked into a CMS and blurring the lines between presentation and logic.

What do you think the best solution here is? could potentially force the closing to a new line? again not the most elegant solution.

{% set posts = craft.entries
    .section("news")
    .orderBy("postDate DESC")
    .limit(10)
    .all() 
%}

FWIW the way that Twig tags can't be embedded within each other it's not quite like trying to find the subtle indentations of nested HTML tags for example. The closing tag is always going to be the next %} or }} so personally I wouldn't find it too jarring if the closing tag is at a different level of indentation to the opening.

@zackad
Copy link
Owner Author

zackad commented Dec 3, 2024

What do you think the best solution here is? could potentially force the closing to a new line? again not the most elegant solution.

I don't know. It's been a while since I work on frontend code. It's hard to judge how it will look like from snippet.

FWIW the way that Twig tags can't be embedded within each other it's not quite like trying to find the subtle indentations of nested HTML tags for example. The closing tag is always going to be the next %} or }} so personally I wouldn't find it too jarring if the closing tag is at a different level of indentation to the opening.

Maybe you're right.

@zackad zackad force-pushed the GH-87/experimental-indentation branch from b24006a to ac06385 Compare December 3, 2024 06:23
@zackad
Copy link
Owner Author

zackad commented Dec 4, 2024

This become more complicated than I originally thought.

Input

{{  craft.entries
    .section("news")
    .section("news")
    .orderBy("postDate DESC")
    .limit(10)
    .all(a.b) }}

Problem 1
Using softline to break the line will produce following output. It will break only when exeed printWidth length. Potentially producing more method chain on same line.

{{
    craft.entries.section('news').section('news').orderBy('postDate DESC')
        .limit(10)
        .all(a.b)
}}

Problem 2
Using hardline will break all member expression regardless they fit in same line or not. This include member expression inside method call parameter.

{{
    craft
        .entries
        .section('news')
        .section('news')
        .orderBy('postDate DESC')
        .limit(10)
        .all(a
            .b)
}}

{# this should fit in a single line #}
{{ craft.entries.all() }}

{# actual output #}
{{
    craft
        .entries
        .all()
}}

@zackad zackad force-pushed the GH-87/experimental-indentation branch from ac06385 to 6a1d8c0 Compare December 18, 2024 07:17
@zackad zackad changed the title [WIP] Experimental: Fix indentation for chained method [WIP] Bugfix: Fix indentation for chained method Dec 18, 2024
@zackad zackad added Printer Logic related on how to print the output Bugfix labels Dec 18, 2024
@zackad zackad force-pushed the GH-87/experimental-indentation branch from 6a1d8c0 to e48f91d Compare December 18, 2024 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bugfix Printer Logic related on how to print the output
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to achieve proper indentation for Twig method chaining?
2 participants