Skip to content

[TwigBridge] Show Twig's loader paths on debug:twig command #24064

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

Closed
wants to merge 3 commits into from

Conversation

yceruto
Copy link
Member

@yceruto yceruto commented Sep 1, 2017

Q A
Branch? 3.4
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

bin/console debug:twig:
twig-loader-paths

This information is not displayed anywhere ATM and it should be important to know:

  • The Twig's namespaces availables
  • The Twig's paths availables
  • The order that templates will be loaded ( regarding its namespace -> LOAD PRIORITY ! )

So it should help us to debug any issue related to circular templates reference, invalid namespaces, unsuccessful attempt to override a template, etc.

WDYT?

@yceruto yceruto changed the title Show Twig's loader paths on debug:twig command [TwigBridge] Show Twig's loader paths on debug:twig command Sep 1, 2017
@yceruto
Copy link
Member Author

yceruto commented Sep 1, 2017

I have another outputs for this list, which do you prefer?

  • (A): @namespace: path (it looks more like how one use it @) (current PR)
  • (B): path: namespace (it looks more like how one define it in twig.paths configuration, yaml's style)
  • (C): path: @namespace (it looks like both A & B together)

@ogizanagi
Copy link
Contributor

For the record, I prefer the current version (A).

@stof
Copy link
Member

stof commented Sep 1, 2017

Outputting namespace first looks better to me (so A), as what is important is the different paths related to a given namespace. Paths used for separate namespaces are irrelevant regarding their order

@yceruto
Copy link
Member Author

yceruto commented Sep 1, 2017

Updated screenshot in description with the latest changes.

@ogizanagi
Copy link
Contributor

ogizanagi commented Sep 1, 2017

I find using * before each path and TableSeparator between each namespace really cluttering. :/

Which of the following outputs would you prefer?

  1. Asterisk bullets, with separator (current)screenshot 2017-09-01 a 21 46 11
  2. No bullet, with separator screenshot 2017-09-01 a 21 46 31
  3. Asterisk bullets, no separator screenshot 2017-09-01 a 21 46 52
  4. Dash bullets, no separator screenshot 2017-09-01 a 21 47 07
  5. No bullet, no separator screenshot 2017-09-01 a 21 47 32
  6. Dash bullets with blank separator (as proposed below)screenshot 2017-09-01 a 21 47 32
  7. Dash bullets with blank separator only if needed (as proposed below)screenshot 2017-09-01 a 22 42 54

My own vote would go to 4 or 5 (but 7's not bad, that's true 😄).

@yceruto
Copy link
Member Author

yceruto commented Sep 1, 2017

and 6 :) (Dash bullets with blank separator) as proposed above :)

  1. Dash bullets with blank separator only if needed.
    7

@yceruto
Copy link
Member Author

yceruto commented Sep 1, 2017

The problem I see with no row separator is that there is no grouping view per namespace, so one could get confused (missing namespace 😕)

@yceruto
Copy link
Member Author

yceruto commented Sep 1, 2017

My vote is for 7.

@ro0NL
Copy link
Contributor

ro0NL commented Sep 1, 2017

What about a --layout=[1-7] option :) And definitely support for layout=random 😂

7 is nice, agree with @yceruto; you getter a better view of path inheritance per namespace.

Tend to prefer (Main) / (Root) over (None) and have it on top.

edit: on 2nd thought (None) is fine :)

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Sep 4, 2017
@fabpot
Copy link
Member

fabpot commented Sep 5, 2017

Thank you @yceruto.

fabpot added a commit that referenced this pull request Sep 5, 2017
…mmand (yceruto)

This PR was squashed before being merged into the 3.4 branch (closes #24064).

Discussion
----------

[TwigBridge] Show Twig's loader paths on debug:twig command

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

`bin/console debug:twig`:
![twig-loader-paths](https://user-images.githubusercontent.com/2028198/29986784-ee5a4094-8f32-11e7-86c8-c78183630221.png
)

This information is not displayed anywhere ATM and it should be important to know:
 * The Twig's namespaces availables
 * The Twig's paths availables
 * The order that templates will be loaded ( regarding its namespace -> LOAD PRIORITY ! )

So it should help us to debug any issue related to circular templates reference, invalid namespaces, unsuccessful attempt to override a template, etc.

WDYT?

Commits
-------

3fdcb40 [TwigBridge] Show Twig's loader paths on debug:twig command
@fabpot fabpot closed this Sep 5, 2017
@yceruto yceruto deleted the twig_loader_paths branch September 5, 2017 20:12
This was referenced Oct 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants