Actually, you can just have spaces in your Yaml keys:

The below problem and solutions no longer apply.


Problem

  1. 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

  1. Rename the property from 'base theme' to 'parent':

    parent: bartik
    

    → "parent" properly encompasses and communicates the underlying parent/child concept.

  2. Update various API method and variable names accordingly. (possibly in a separate issue)

Proposed solution D

  1. Rename the property from 'base theme' to 'parent_theme':

    parent_theme: bartik
    

    → "parent" may be ambiguous.

  2. Update various API method and variable names accordingly. (possibly in a separate issue)


Obsolete options

Proposed solution A

  1. Rename the property from 'base theme' to just 'base':

    base: bartik
    

    → Just get rid of the offending space.

Proposed solution C

  1. Rename the property from 'base theme' into 'base_theme':

    base_theme: bartik
    

    → Just replace the offending space with an underscore.

Files: 
CommentFileSizeAuthor
#48 2228125.patch2.28 KBdawehner
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 77,600 pass(es). View
#47 2228125-47.patch10.92 KBlauriii
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 77,600 pass(es). View
#20 interdiff.txt797 bytesjsbalsera
#20 Rename_yml_info_file_property_base_theme_to_base-2228125-20.patch12.24 KBjsbalsera
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 67,604 pass(es). View
#16 Rename_yml_info_file_property_base_theme_to_base-2228125-16.patch12.14 KBjsbalsera
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 67,465 pass(es). View
#6 Rename_yml_info_file_property_base_theme_to_base-2228125-6.patch12.14 KBjsbalsera
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch Rename_yml_info_file_property_base_theme_to_base-2228125-6.patch. Unable to apply patch. See the log in the details link for more information. View
#3 Rename_yml_info_file_property_base_theme_to_base-2228125.patch12.53 KBbudda
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch Rename_yml_info_file_property_base_theme_to_base-2228125.patch. Unable to apply patch. See the log in the details link for more information. View

Comments

sun’s picture

Title: It is not good format for declare base theme in yml » 'base theme' property in theme .info.yml files needs to be wrapped in quotes
Issue tags: -yml, -theming +DX (Developer Experience), +TX (Themer Experience)

Yeah, 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")

budda’s picture

Changing to "base" looks cleaner in the yml file whilst still being clear to the developer/themer.

budda’s picture

FileSize
12.53 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch Rename_yml_info_file_property_base_theme_to_base-2228125.patch. Unable to apply patch. See the log in the details link for more information. View

An 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.

sun’s picture

Status: Active » Needs review

Status: Needs review » Needs work
jsbalsera’s picture

Status: Needs work » Needs review
FileSize
12.14 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch Rename_yml_info_file_property_base_theme_to_base-2228125-6.patch. Unable to apply patch. See the log in the details link for more information. View

The patch was created with core as base directory, so I simply created it again.

Status: Needs review » Needs work
sun’s picture

Interesting — 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?

Codenator’s picture

Only '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'

sun’s picture

Status: Needs work » Needs review
budda’s picture

@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.

jibran’s picture

jibran’s picture

Status: Needs review » Reviewed & tested by the community

It is only replacing 'base theme' property with base and we have test if this come back green then it is RTBC.

Status: Reviewed & tested by the community » Needs work
jibran’s picture

Issue tags: +Needs change notification

And we need change notice for this as well.

jsbalsera’s picture

Status: Needs work » Needs review
FileSize
12.14 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 67,465 pass(es). View

Reroll

Cottser’s picture

Issue tags: +Twig
sun’s picture

+++ b/core/includes/theme.inc
@@ -124,7 +124,7 @@ function drupal_theme_initialize() {
- *    An optional array of objects that represent the 'base theme' if the
+ *    An optional array of objects that represent the 'base' if the
  *    theme is meant to be derivative of another theme. It requires

Let's re-add "theme" (sans quotes) after 'base' to not break this sentence. :-)

mortendk’s picture

from a theme persepctive using base: instead of base theme: in the yml file makes perfect sense - so no worries there

jsbalsera’s picture

FileSize
12.24 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 67,604 pass(es). View
797 bytes

Changing the comment.

sun’s picture

Status: Needs review » Reviewed & tested by the community

Thanks! :-)

jibran’s picture

Issue tags: -Needs change notification

Added a change notice draft. https://drupal.org/node/2234253

webchick’s picture

Status: Reviewed & tested by the community » Needs review

Hm. 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.

sun’s picture

Issue summary: View changes

If 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.

shahinam’s picture

Just 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'

mortendk’s picture

just 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 ;)

webchick’s picture

Status: Needs review » Reviewed & tested by the community

Ok, 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").

sun’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Needs review

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

"base theme" / "base" is understood by any drupal themer

Is it possible that Drupal themers only understand what it is, because they once had to learn it the hard way? :-)

sun’s picture

Issue summary: View changes
markcarver’s picture

I 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".

SeriousMatters’s picture

another vote for "parent_theme", precise and unambiguous.

sun’s picture

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

Amber Himes Matz’s picture

Another +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. :)

shahinam’s picture

+1 for _theme suffix

Both base_theme and parent_theme are good, makes sense for existing themers and easy to understand for new ones.

alexrayu’s picture

+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.

jsbalsera’s picture

+1 to parent_theme, I agree with alexrayu completely.

holist’s picture

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

sun’s picture

Issue summary: View changes

Looks 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.

markcarver’s picture

Agreed parent sounds more appropriate. _theme is extraneous and unnecessary. It's already declared as type: theme and rather apparent in which context this belongs.

Codenator’s picture

+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.

cweagans’s picture

Status: Needs review » Needs work

Looks like people are generally in favor of "parent". Moving back to CNW for continued work on the patch.

markcarver’s picture

Issue tags: +sprint, +focus
markcarver’s picture

pingers’s picture

If 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.

Cottser’s picture

Bump. Can we rename the YAML key and do the variable/method/etc. renaming in a followup? Beta is looming.

lauriii’s picture

Assigned: Unassigned » lauriii

I 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'

lauriii’s picture

Assigned: lauriii » Unassigned
Status: Needs work » Needs review
FileSize
10.92 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 77,600 pass(es). View
dawehner’s picture

FileSize
2.28 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 77,600 pass(es). View

Just 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.

name: 'Breakpoint test theme'
type: theme
description: 'Test theme for breakpoint.'
version: VERSION
core: 8.x
base theme: bartik

Proove: Patch.

webchick’s picture

Oh! :) Lovely.

Cottser’s picture

Title: 'base theme' property in theme .info.yml files needs to be wrapped in quotes » 'base theme' property in theme .info.yml files does not need to be wrapped in quotes
Category: Bug report » Task
Priority: Major » Normal
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs issue summary update

So this seems more appropriate then. Added the best link I could find to the top of the issue summary.

Thank you @dawehner!

Cottser’s picture

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

  • webchick committed 5b11264 on 8.0.x
    Issue #2228125 by dawehner: 'base theme' property in theme .info.yml...
Codenator’s picture

So now base theme not parent ?
I not understand

Codenator’s picture

patch #47 for parent and path #48 just test for base theme

Cottser’s picture

@Codenator - no change was made to the structure of the .info.yml file for themes because the quotes are simply not necessary.

Cottser’s picture

So the patch in #48 simply removes the quotes.

Codenator’s picture

OK

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.