Actually, you can just have spaces in your Yaml keys:
- Explanation: http://yaml.org/spec/1.2/spec.html#id2786942 (7.3 Flow Scalar Styles and 7.3.3 Plain Style)
- Clear example: http://yaml.org/spec/1.2/spec.html#id2760372
The below problem and solutions no longer apply.
Problem
-
Since the
'base theme'
.info.yml property contains a space, it has to be specified like this:'base theme': bartik
cf. https://drupal.org/node/2165673
That's ugly.
Proposed solution B
-
Rename the property from
'base theme'
to'parent'
:parent: bartik
→ "parent" properly encompasses and communicates the underlying parent/child concept.
-
Update various API method and variable names accordingly. (possibly in a separate issue)
Proposed solution D
-
Rename the property from
'base theme'
to'parent_theme'
:parent_theme: bartik
→ "parent" may be ambiguous.
-
Update various API method and variable names accordingly. (possibly in a separate issue)
Obsolete options
Proposed solution A
-
Rename the property from
'base theme'
to just'base'
:base: bartik
→ Just get rid of the offending space.
Proposed solution C
-
Rename the property from
'base theme'
into'base_theme'
:base_theme: bartik
→ Just replace the offending space with an underscore.
Comment | File | Size | Author |
---|---|---|---|
#48 | 2228125.patch | 2.28 KB | dawehner |
#47 | 2228125-47.patch | 10.92 KB | lauriii |
#20 | interdiff.txt | 797 bytes | jsbalsera |
#20 | Rename_yml_info_file_property_base_theme_to_base-2228125-20.patch | 12.24 KB | jsbalsera |
#16 | Rename_yml_info_file_property_base_theme_to_base-2228125-16.patch | 12.14 KB | jsbalsera |
Comments
Comment #1
sunYeah, agreed, this is a problem.
Let's simply rename this to "base_theme".
Ideally, I'd even go one step further and rename it to just "base".
("parent" would be much more accurate, but unfortunately, that would invalidate tons of theme system documentation that currently refers to "base theme")
Comment #2
buddaChanging to "base" looks cleaner in the yml file whilst still being clear to the developer/themer.
Comment #3
buddaAn initial crack to moving the property name from 'base theme' to 'base'. Taking in to account the core themes, yml processing and all associated tests.
Giving PHPStorm a go for creating patches. Fingers crossed.
Comment #4
sunComment #6
jsbalseraThe patch was created with core as base directory, so I simply created it again.
Comment #8
sunInteresting — this patch somehow triggers the same weird test failures I also got in #2228201: Replace global $theme with global $theme_info
Unfortunately, I wasn't able to figure out why they are happening... Can you debug?
Comment #9
Codenator CreditAttribution: Codenator commentedOnly 'base' going confuse in code just better add to base theme '_' so 'base_theme' .
Because just base base what? controller? from where I think base_theme is best way to help for coders understood what this mean.
Thanks.
p.s. But I prefer 'parent_theme'
Comment #10
sun6: Rename_yml_info_file_property_base_theme_to_base-2228125-6.patch queued for re-testing.
Comment #11
budda@codenator if 'base' appears in the theme info yaml file then it has some context about its purpose.
But 'parent' is an option i like a bit too.
Comment #12
jibran6: Rename_yml_info_file_property_base_theme_to_base-2228125-6.patch queued for re-testing.
Comment #13
jibranIt is only replacing
'base theme'
property withbase
and we have test if this come back green then it is RTBC.Comment #15
jibranAnd we need change notice for this as well.
Comment #16
jsbalseraReroll
Comment #17
star-szrTagging for http://drupaltwig.org.
Comment #18
sunLet's re-add "theme" (sans quotes) after 'base' to not break this sentence. :-)
Comment #19
mortendk CreditAttribution: mortendk commentedfrom a theme persepctive using base: instead of base theme: in the yml file makes perfect sense - so no worries there
Comment #20
jsbalseraChanging the comment.
Comment #21
sunThanks! :-)
Comment #22
jibranAdded a change notice draft. https://drupal.org/node/2234253
Comment #23
webchickHm. If we're going to break this anyway, it sounded like more people were in favour of "parent" than "base." This also makes a lot more sense to me. Looks like WordPress uses somewhat this same terminology as well: https://codex.wordpress.org/Child_Themes Versus I was not able to find a similar concept in any other system by googling for "base theme."
I think invaliding some documentation is probably the least of our worries, since there's basically nothing from D7 that holds true for D8 anymore anyway. ;)
Knocking back to needs review to see what people think.
Comment #24
sunIf renaming it to 'parent' is an option, then I think I'd be in favor of that, yeah. "Parent" concisely describes the model and algorithm under the hood, and that concept is generally understood.
In that case, I'd suggest we just care for the .info.yml property rename here + perform a whole bunch of API method + variable renames in a separate issue.
Rewrote the issue summary to properly reflect the problem space and proposed solutions.
Comment #25
shahinam CreditAttribution: shahinam commentedJust a thought!
Why just not make it base_theme like all other keys.
This preserves the well known term 'base theme' instead of new one 'parent'
Comment #26
mortendk CreditAttribution: mortendk commentedjust to chime in here (not that i feel strongly about) but a "base theme" / "base" is understood by any drupal themer, introducing new names for known concepts makes little sense to me - else can I wish that we rename blocks, so we can free that name for twig ;)
Comment #27
webchickOk, fair enough. Let's go with this then. While there are underscores in certain .info.yml properties (notably "ckeditor_stylesheets," which is a bit bizarre in and of itself) the norm is just single words, even if they havetobecrammedtogether (like "stylesheets").
Comment #28
sunI guess this needs to happen before beta, but I'd like to gather some more feedback first (and just asked in IRC #drupal-themes + #drupal).
I'm also adding "base_theme" as proposal C to the summary, since it has been mentioned a few times by now.
re: @mortendk:
Is it possible that Drupal themers only understand what it is, because they once had to learn it the hard way? :-)
Comment #29
sunComment #30
markhalliwellI think we should make the shift now to the "parent/child/grandchild" paradigm. It would help clarify a whole lot in documentation, especially when dealing or discussing "base theme/sub-theme/sub-sub-theme".
Comment #31
SeriousMatters CreditAttribution: SeriousMatters commentedanother vote for "parent_theme", precise and unambiguous.
Comment #32
sun@SeriousMatters: Was that a vote in favor of proposal B, or are you suggesting a new option D to rename
'base theme'
into'parent_theme'
? (same as B, but with additional _theme suffix)Comment #33
Amber Himes MatzAnother +1 for parent (proposed solution B).
Yes, Drupal themers know what a base theme is, but I think the parent/child concept is easy enough to understand and explain, maybe even easier to understand, especially for newcomers to the platform. For example, the concept of inheritance makes more sense in the context of a parent/child relationship than it does in the context of base theme/sub-theme relationship, because everyone understands that a child inherits traits from a parent but also has unique traits of her own. But how do a "base" and a "sub" relate? Subterraineously? See, that's not even a word. Although, I will concede that base theme is a term that should be familiar to Drupal themers already, even if they did learn it "the hard way."
@webchick makes a good point about Wordpress having the concept of child themes already. There are a lot of folks that build sites using both Wordpress and Drupal, so I think it's actually kind of nice to introduce even the tiniest bit of consistency between the two platforms. :)
Comment #34
shahinam CreditAttribution: shahinam commented+1 for _theme suffix
Both base_theme and parent_theme are good, makes sense for existing themers and easy to understand for new ones.
Comment #35
alexrayu CreditAttribution: alexrayu commented+1 For parent. I would rather have it as parent_theme, but parent is next in line.
I would still keep the _theme suffix, since base or parent without that suffix makes me think about some Drupal component, like theming engine, rather than another theme.
Comment #36
jsbalsera+1 to parent_theme, I agree with alexrayu completely.
Comment #37
holist CreditAttribution: holist commented+1 for parent. Makes more sense than base when there are more two (or more) levels of inheritance. Adding _theme would be redundant, I think (the parent of a wombat is a wombat).
Comment #38
sunLooks like all options regarding "base" are off the table now. Updated issue summary accordingly.
Remaining options:
B) "parent"
D) "parent_theme"
Personally, the _theme suffix looks superfluous to me. I don't see how a (top-level) "parent" property could be ambiguous.
FWIW, we also have "engine" (which defaults to 'twig'), which equally does not have a _theme suffix. Likewise, "name" is not "theme_name" either. All of the top-level properties imply the scope of a "theme".
Therefore, just "parent" seems to be sufficient and most accurate.
Comment #39
markhalliwellAgreed
parent
sounds more appropriate._theme
is extraneous and unnecessary. It's already declared astype: theme
and rather apparent in which context this belongs.Comment #40
Codenator CreditAttribution: Codenator commented+1 to parent_theme (proposal D)
It is make sense because for newcomers and exiting people in Drupal it is easy to understand and remember what that mean exactly.
Comment #41
cweagansLooks like people are generally in favor of "parent". Moving back to CNW for continued work on the patch.
Comment #42
markhalliwellComment #43
markhalliwellComment #44
pingers CreditAttribution: pingers commentedIf we change to parent, there's a bunch of other uses of $base_theme, $base_theme_info, $theme_object->base_themes etc that should(maybe) also be changed... not a small task. Otherwise we end up with 'parent' in one place, 'base theme' in another. One or the other please.
Comment #45
star-szrBump. Can we rename the YAML key and do the variable/method/etc. renaming in a followup? Beta is looming.
Comment #46
lauriiiI think we should fix all the stuff needed for beta sooner than later in order to get it in. We can take care of the rest later.
I will create a patch for this with key 'parent'
Comment #47
lauriiiComment #48
dawehnerJust a general question. Did someone considered that the following code IS a valid YMl, see #2328411-14: Convert all permissions to yml files and permission callbacks
Proove: Patch.
Comment #49
webchickOh! :) Lovely.
Comment #50
star-szrSo this seems more appropriate then. Added the best link I could find to the top of the issue summary.
Thank you @dawehner!
Comment #51
star-szrAdding the link from #2328411-14: Convert all permissions to yml files and permission callbacks also.
Comment #52
webchickCommitted and pushed to 8.x. Thanks!
Comment #54
Codenator CreditAttribution: Codenator commentedSo now
base theme
notparent
?I not understand
Comment #55
Codenator CreditAttribution: Codenator commentedpatch #47 for
parent
and path #48 just test forbase theme
Comment #56
star-szr@Codenator - no change was made to the structure of the .info.yml file for themes because the quotes are simply not necessary.
Comment #57
star-szrSo the patch in #48 simply removes the quotes.
Comment #58
Codenator CreditAttribution: Codenator commentedOK