Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
yceruto wants to merge3 commits intosymfony:3.4fromyceruto:twig_loader_paths

Conversation

@yceruto
Copy link
Member

@ycerutoyceruto commentedSep 1, 2017
edited
Loading

QA
Branch?3.4
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets-
LicenseMIT
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?

julienfalque, ogizanagi, ro0NL, Koc, flo-sch, and yceruto reacted with thumbs up emoji
@ycerutoyceruto changed the titleShow Twig's loader paths on debug:twig command[TwigBridge] Show Twig's loader paths on debug:twig commandSep 1, 2017
@yceruto
Copy link
MemberAuthor

yceruto commentedSep 1, 2017
edited
Loading

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 intwig.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).

PaperCoder, yceruto, and ro0NL reacted with thumbs up emoji

@stof
Copy link
Member

stof commentedSep 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
MemberAuthor

Updated screenshot in description with the latest changes.

@ogizanagi
Copy link
Contributor

ogizanagi commentedSep 1, 2017
edited
Loading

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 separatorscreenshot 2017-09-01 a 21 46 31
  3. Asterisk bullets, no separatorscreenshot 2017-09-01 a 21 46 52
  4. Dash bullets, no separatorscreenshot 2017-09-01 a 21 47 07
  5. No bullet, no separatorscreenshot 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
MemberAuthor

yceruto commentedSep 1, 2017
edited
Loading

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

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

@yceruto
Copy link
MemberAuthor

yceruto commentedSep 1, 2017
edited
Loading

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
MemberAuthor

yceruto commentedSep 1, 2017
edited
Loading

My vote is for 7.

@ro0NL
Copy link
Contributor

ro0NL commentedSep 1, 2017
edited
Loading

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 :)

ogizanagi, yceruto, and chalasr reacted with laugh emoji

@nicolas-grekasnicolas-grekas added this to the3.4 milestoneSep 4, 2017
@fabpot
Copy link
Member

Thank you@yceruto.

yceruto reacted with hooray emoji

fabpot added a commit that referenced this pull requestSep 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
@fabpotfabpot closed thisSep 5, 2017
@ycerutoyceruto deleted the twig_loader_paths branchSeptember 5, 2017 20:12
This was referencedOct 18, 2017
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@chalasrchalasrchalasr left review comments

@fabpotfabpotfabpot approved these changes

+1 more reviewer

@ogizanagiogizanagiogizanagi left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

3.4

Development

Successfully merging this pull request may close these issues.

8 participants

@yceruto@ogizanagi@stof@ro0NL@fabpot@chalasr@nicolas-grekas@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp