tl;dr Let's adopt a better structure for skins. A more detailed proposal is at the bottom.
As you might know, I am doing a Google Summer of Code project aiming to disentangle the mess of MediaWiki's skinning system a little bit, make creating custom skins a bit less painful and improve the separation between MediaWiki and its core skins [0] (https://www.mediawiki.org/wiki/Separating_skins_from_core_MediaWiki).
I want this thread to result in code changes :)
----
So, MediaWiki supports skins, and apart from the four core ones there's a couple dozen of skins available for installation [1]. And looking at them, it seems as if every single one used a different directory structure, and this a different installation method.
I think this is bad, and that we should standardize on something – preferably one of the widely used methods – and use it for the core skins as well to provide a good example.
----
There seem to be three popular ways:
* $IP/skins/SkinName.php for the main file plus $IP/skins/skinname/ for assets, using an autodiscovery mechanism to automagically make the skin available after the files are copied in the right place. This is used by all of the core skins (Vector has some special cases, but let's ignore that for now), as well as many external skins (e.g. Cavendish [2]), at a glance mostly older ones. * $IP/skins/SkinName/ for both assets and PHP files ($IP/skins/skinname/SkinName.php etc.), using require_once in LocalSettings like extensions to load the skin, manually adding an entry to $wgValidSkinNames in the main PHP file. This seems to be the preferred method among "modern" skins, for example Erudite [3] or Nimbus [4]. * $IP/extensions/SkinName/ for everything, the rest as above. This makes the skin work exactly like an extension. The only example I could find on mediawiki.org is the Nostalgia skin [5].
----
The first one sounds like a no-go for me (in spite of being currently used for core skins, ugh):
* The directory structure makes it annoying to both manage and write such skins (you need to copy/delete the PHP file and the directory separately, many text editors provide additional features for files contained in a single directory, and just look at our .gitignore file for skins oh god why [6]). * The usage of autodiscovery, while making installation and testing a bit simpler, makes it impossible or unpleasant to temporarily disable a skin or to provide configuration settings for it (the last point doesn't affect core skins).
This leaves us with the two latter options: packaging skins similarly to extensions and sticking them in /skins, or packaging them like extensions and treating them like extensions. These two options are pretty similar and discussing them will be a bit bikesheddy, but let's do it anyway. (Note also that even if we wanted to, we can't stop anyone from using either of these if they feel like it, as MediaWiki supports loading everything from anywhere if you really want. We can, however, deprecate skin autodiscovery.)
----
Personally I'm leading towards the /skins/SkinName option. The pros are:
* It seems to be more widely used, which means that it "felt right" to a lot of people, and that shouldn't be underestimated. * It's less revolutionary, and rather a simple improvement over the current system. * It's more intuitive when compared to how other applications / projects works. (Corollary: just because MediaWiki skins can do everything that extensions can do, we shouldn't encourage that.) * Since it's still similar to how extensions work, adapting the current system (WMF deployments, tarball packaging, installation via web installer) should be straightforward. * Switching current skins to this system within the mediawiki/core repo will be trivial.
The pros of using /extensions/SkinName are:
* We already have a battle-tested system for doing things with extensions (WMF deployments, tarball packaging, installation via web installer). * All non-core code in one place.
I would like to settle this within a week or two. Help! :)
Thoughts?
I will document the result and, if feasible, convert core skins to be closer to the recommended format afterwards.
----
tl;dr Let's start putting all skins files in a single directory, and let's use a grown-up structure with one class per file + separate init code for them. Okay?
[1] https://www.mediawiki.org/wiki/Category:Skin (this category tree is a mess, huh) [2] https://www.mediawiki.org/wiki/Skin:Cavendish [3] https://www.mediawiki.org/wiki/Skin:Erudite [4] https://www.mediawiki.org/wiki/Skin:Nimbus [5] https://www.mediawiki.org/wiki/Extension:Nostalgia [6] https://git.wikimedia.org/blob/mediawiki%2Fcore/master/skins%2F.gitignore
On Tue, May 20, 2014 at 5:25 PM, Bartosz Dziewoński matma.rex@gmail.comwrote:
tl;dr Let's start putting all skins files in a single directory, and let's use a grown-up structure with one class per file + separate init code for them. Okay?
I wouldn't consider this change to be truly revolutionary. Would would really be a great restructuring of the skinning system is if I could make a skin by just writing a couple of HTML templates (probably using Gabriel's DOM-based templating language), throw them in a directory and then tell MediaWiki where the directory is.
However, staying on the topic that you brought up, I do agree with keeping skins in a separate directory than extensions. It implies a bit of control on our part concerning how skins need to be structured, whereas if they were extensions, we cannot place any requirements on the structure.
*-- * *Tyler Romeo* Stevens Institute of Technology, Class of 2016 Major in Computer Science
On Tue, 20 May 2014 23:38:14 +0200, Tyler Romeo tylerromeo@gmail.com wrote:
I wouldn't consider this change to be truly revolutionary. Would would really be a great restructuring of the skinning system is if I could make a skin by just writing a couple of HTML templates (probably using Gabriel's DOM-based templating language), throw them in a directory and then tell MediaWiki where the directory is.
It is not meant to be. I prefer moving forward in small steps :)
However, there is an ambitious RFC on this very topic in progress https://www.mediawiki.org/wiki/Requests_for_comment/Redo_skin_framework. Enjoy.
On Tue, May 20, 2014 at 2:25 PM, Bartosz Dziewoński matma.rex@gmail.comwrote:
tl;dr Let's start putting all skins files in a single directory, and let's use a grown-up structure with one class per file + separate init code for them. Okay?
Sounds good to me. I agree with Tyler that there's more to wish for, but what you're proposing doesn't at all foreclose further innovation, so I think it's a practical way forward.
On Wed, May 21, 2014 at 12:25 AM, Bartosz Dziewoński matma.rex@gmail.comwrote:
As you might know, I am doing a Google Summer of Code project aiming to disentangle the mess of MediaWiki's skinning system a little bit, make creating custom skins a bit less painful and improve the separation between MediaWiki and its core skins [0] (https://www.mediawiki.org/ wiki/Separating_skins_from_core_MediaWiki).
The skin system is likely rather intimidating for a newer developer; I know my way around it, and so do you and some other core MediaWiki developers, but we're not the target audience, since MW's default skin changes *very* rarely. As such, it'd be beneficial to everyone to make the skin system a bit friendlier and less of a gigantic mess, which is what it mostly is today.
- $IP/skins/SkinName.php for the main file plus $IP/skins/skinname/ for
assets, using an autodiscovery mechanism to automagically make the skin available after the files are copied in the right place. This is used by all of the core skins (Vector has some special cases, but let's ignore that for now), as well as many external skins (e.g. Cavendish [2]), at a glance mostly older ones.
I'd say that this method is a remnant from darker ages and it'd be nice if we could forget it ever existed...
- $IP/skins/SkinName/ for both assets and PHP files
($IP/skins/skinname/SkinName.php etc.), using require_once in LocalSettings like extensions to load the skin, manually adding an entry to $wgValidSkinNames in the main PHP file. This seems to be the preferred method among "modern" skins, for example Erudite [3] or Nimbus [4].
I'm going to assume that the lowercase "skinname" in $IP/skins/skinname/SkinName.php is just a typo and you meant it to be CamelCased as "SkinName". If and when so, yes, this is what should be our recommended way of doing it. CamelCase is how we name things consisting of multiple words (i.e. BlueSky, DuskToDawn, HowTo, ...), so it's only reasonable to use CamelCase here too. Having written and cleaned up many skins myself, this is the naming convention I prefer and that seems natural right from the start.
- The usage of autodiscovery, while making installation and testing a bit
simpler, makes it impossible or unpleasant to temporarily disable a skin or to provide configuration settings for it (the last point doesn't affect core skins).
This is why autodiscovery needs to go.
This leaves us with the two latter options: packaging skins similarly to extensions and sticking them in /skins, or packaging them like extensions and treating them like extensions. These two options are pretty similar and discussing them will be a bit bikesheddy, but let's do it anyway.
Bikeshedding? In wikitech-l? You must be new here. ;-)
The pros of using /extensions/SkinName are:
[...]
* All non-core code in one place.
While this is true and somewhat handy, there can be unexpected situations such as two independent developers (or teams) coming up with two different things that have...the exact same name -- this is true for "Nimbus"; there is an extension and a skin with that name; the skin likely predates the extension, but unlike the extension, the skin wasn't always FOSS.
Thanks and regards, -- Jack Phoenix MediaWiki developer
On Wed, 21 May 2014 00:06:57 +0200, Jack Phoenix jack@countervandalism.net wrote:
The pros of using /extensions/SkinName are:
[...]
- All non-core code in one place.
While this is true and somewhat handy, there can be unexpected situations such as two independent developers (or teams) coming up with two different things that have...the exact same name -- this is true for "Nimbus"; there is an extension and a skin with that name; the skin likely predates the extension, but unlike the extension, the skin wasn't always FOSS.
We actually also have a Vector extension (thankfully discontinued now) and a Vector skin (and both were made by the same team, too! ;) ).
On 2014-05-20, 2:25 PM, Bartosz Dziewoński wrote:
There seem to be three popular ways:
- $IP/skins/SkinName.php for the main file plus $IP/skins/skinname/
for assets, using an autodiscovery mechanism to automagically make the skin available after the files are copied in the right place. This is used by all of the core skins (Vector has some special cases, but let's ignore that for now), as well as many external skins (e.g. Cavendish [2]), at a glance mostly older ones.
I wouldn't say it's popular. It's just the way it was done in core and the only way anyone without knowledge of large swaths of MediaWiki could figure out.
- $IP/skins/SkinName/ for both assets and PHP files
($IP/skins/skinname/SkinName.php etc.), using require_once in LocalSettings like extensions to load the skin, manually adding an entry to $wgValidSkinNames in the main PHP file. This seems to be the preferred method among "modern" skins, for example Erudite [3] or Nimbus [4].
You're misstating this pattern, the pattern goes:
* $IP/skins/skinname/skinname.php * $IP/skins/skinname/* (assets)
The directory name and entry file are both lowercase, not skins/SkinName/ nor skins/skinname/SkinName, this means:
* The skinname you require is the same one assigned to $wgValidSkins and set on $wgDefaultSkin and set on $skinname, and $stylepath. * For all skins using the old autoload pattern the assets directory remains them same, so nothing has to be re-cached. * The same "entry filename = dirname + .php" pattern we've worked to make extensions follow is followed by skins, which will also help implement special loading for skins.
This is the pattern described by the tutorial I wrote, used by Erudite, monaco-port, every custom skin I've developed for a client, and the pattern that should be used going forward. I had hoped to eventually add special behaviors in the loading of skins that would reduce the boilerplate needed to setup skins following this pattern.
- $IP/extensions/SkinName/ for everything, the rest as above. This
makes the skin work exactly like an extension. The only example I could find on mediawiki.org is the Nostalgia skin [5].
This was used for Nostalgia because the cluster didn't have any handling for $IP/skins/ like it does for $IP/extensions/. The only thing that should be using Nostalgia is https://nostalgia.wikipedia.org/. And no other skin should be using the pattern Nostalgia uses.
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://danielfriesen.name/]
On 20/05/14 22:07, Daniel Friesen wrote:
On 2014-05-20, 2:25 PM, Bartosz Dziewoński wrote:
There seem to be three popular ways:
- $IP/skins/SkinName.php for the main file plus $IP/skins/skinname/
for assets, using an autodiscovery mechanism to automagically make the skin available after the files are copied in the right place. This is used by all of the core skins (Vector has some special cases, but let's ignore that for now), as well as many external skins (e.g. Cavendish [2]), at a glance mostly older ones.
I wouldn't say it's popular. It's just the way it was done in core and the only way anyone without knowledge of large swaths of MediaWiki could figure out.
- $IP/skins/SkinName/ for both assets and PHP files
($IP/skins/skinname/SkinName.php etc.), using require_once in LocalSettings like extensions to load the skin, manually adding an entry to $wgValidSkinNames in the main PHP file. This seems to be the preferred method among "modern" skins, for example Erudite [3] or Nimbus [4].
You're misstating this pattern, the pattern goes:
- $IP/skins/skinname/skinname.php
- $IP/skins/skinname/* (assets)
The directory name and entry file are both lowercase, not skins/SkinName/ nor skins/skinname/SkinName, this means:
- The skinname you require is the same one assigned to $wgValidSkins and set on $wgDefaultSkin and set on $skinname, and $stylepath.
- For all skins using the old autoload pattern the assets directory remains them same, so nothing has to be re-cached.
- The same "entry filename = dirname + .php" pattern we've worked to make extensions follow is followed by skins, which will also help implement special loading for skins.
This is the pattern described by the tutorial I wrote, used by Erudite, monaco-port, every custom skin I've developed for a client, and the pattern that should be used going forward. I had hoped to eventually add special behaviors in the loading of skins that would reduce the boilerplate needed to setup skins following this pattern.
- $IP/extensions/SkinName/ for everything, the rest as above. This
makes the skin work exactly like an extension. The only example I could find on mediawiki.org is the Nostalgia skin [5].
This was used for Nostalgia because the cluster didn't have any handling for $IP/skins/ like it does for $IP/extensions/. The only thing that should be using Nostalgia is https://nostalgia.wikipedia.org/. And no other skin should be using the pattern Nostalgia uses.
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://danielfriesen.name/]
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Out of curiosity, what are your reasons for advocating lowercase skin names over the standard camelcase format used throughout the rest of MediaWiki?
-I
On 2014-05-20, 3:48 PM, Isarra Yos wrote:
Out of curiosity, what are your reasons for advocating lowercase skin names over the standard camelcase format used throughout the rest of MediaWiki?
I included those in my bullet points.
* The skinname you require is the same one assigned to $wgValidSkins and set on $wgDefaultSkin and set on $skinname, and $stylepath. * For all skins using the old autoload pattern the assets directory remains them same, so nothing has to be re-cached.
There's also a third reason.
There are 3 names for a skin containing two words:
* foobar * FooBar * Foo Bar
Foo Bar is exposed to users in the interface as the human readable name. And foobar is exposed in &useskin= $wgDefaultSkin and the current directory asset structure, in essence it IS the canonical name of the skin.
FooBar however generally isn't exposed anywhere. It exists exclusively for things like skin class names like SkinFooBar and optionally some filenames, both of which are internal to the skin.
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://danielfriesen.name/]
On 20/05/14 23:05, Daniel Friesen wrote:
On 2014-05-20, 3:48 PM, Isarra Yos wrote:
Out of curiosity, what are your reasons for advocating lowercase skin names over the standard camelcase format used throughout the rest of MediaWiki?
I included those in my bullet points.
* The skinname you require is the same one assigned to $wgValidSkins and set on $wgDefaultSkin and set on $skinname, and $stylepath. * For all skins using the old autoload pattern the assets directory remains them same, so nothing has to be re-cached.
But that sounds more like an oversight with the autoloader implementation and class setup than an actual reason why it would be good practice to use lowercase. Was there a particular design reason to set those up to be that way?
There's also a third reason.
There are 3 names for a skin containing two words:
- foobar
- FooBar
- Foo Bar
Foo Bar is exposed to users in the interface as the human readable name. And foobar is exposed in &useskin= $wgDefaultSkin and the current directory asset structure, in essence it IS the canonical name of the skin.
FooBar however generally isn't exposed anywhere. It exists exclusively for things like skin class names like SkinFooBar and optionally some filenames, both of which are internal to the skin.
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://danielfriesen.name/]
Two of those also apply to extensions in general - FooBar and Foo Bar. Has this sort of inconsistency between name presentations ever posed a problem there? And with skins in particular, why would the internal handling of the skin name be coupled to the filename at all? And why the case-sensitivity, especially when not all platforms that mw supports even use case-sensitive filesystems? Is this just a holdover from the autoloader implementation?
Sorry for all the questions, but this all seems quite odd, and I never really had the opportunity to put much thought into it before.
-I
On 2014-05-20, 10:28 PM, Isarra Yos wrote:
On 20/05/14 23:05, Daniel Friesen wrote:
* The skinname you require is the same one assigned to $wgValidSkins and set on $wgDefaultSkin and set on $skinname, and $stylepath. * For all skins using the old autoload pattern the assets directory remains them same, so nothing has to be re-cached.But that sounds more like an oversight with the autoloader implementation and class setup than an actual reason why it would be good practice to use lowercase. Was there a particular design reason to set those up to be that way?
I'm not sure which item, you think you're replying to but you might be misunderstanding what I'm saying.
The key used for $wgValidSkins/$wgDefaultSkin/$skinname has always been lowercase, and I haven't seen anyone suggesting that we start using &useskin=MonoBook, which would be a bad idea since this key is used in i18n message keys and the skin- css class – places you don't want to use upper case letters. Plus of course using upper case letters there is just asking for a pile of brand new bugs as someone accidentally uses the wrong case and hard to debug errors start popping up.
And for the bit on re-caching that is simple. Assets for core skins like Vector and old skins that are implemented using the autoloader are currently at: {mediawiki}/skins/vector/* If you change the directory structure so we load $IP/skins/Vector/Vector.php (instead of $IP/skin/vector/vector.php) then the path to assets becomes: {mediawiki}/skins/Vector/*
Even though not a single one of the assets have changed the URL has, so, re-cache. ResourceLoader moves most CSS, JS, and some images to load.php but there are still resources that are referred to with an actual URL, like Vector's .htc file, all css images in IE without data: URI support, css images that aren't marked with @embed (because they're too big, use multi-backgrounds, etc...), anything in an <img> using $skin->getSkinStylePath(...), and any stylesheet still loaded with addStyle (IE stylesheets, etc...).
Two of those also apply to extensions in general - FooBar and Foo Bar. Has this sort of inconsistency between name presentations ever posed a problem there? And with skins in particular, why would the internal handling of the skin name be coupled to the filename at all? And why the case-sensitivity, especially when not all platforms that mw supports even use case-sensitive filesystems? Is this just a holdover from the autoloader implementation?
Two is fine, the problem is having three, or rather in essence inventing a third just for skins.
Extensions have two names visible outside of the internal API of the extension:
* FooBar is a canonical key, it's used in the dirname (plus the entry file name) and "usually" (there is some inconsistency) the 'name' key to extension credits (. * Foo Bar is the human name, and well, sometimes it doesn't even exist even when an extension name has multiple words.
There is some inconsistency in what we use in extension credits, we have a 'name' key and sometimes a human name gets filled in there ('PDF Handler', 'Central Auth', 'Abuse Filter', ...) and in others the canonical key gets used (ParserFunctions, SyntaxHighlight, AntiBot, TorBlock, MobileFrontend, ...), the most common behavior seems to be to just use the canonical key, giving most extensions only one name. If we were serious about human names for extensions we would make it possible to i18n them.
Skins are different, they have two names visible outside of the internal API of the skin:
* foobar is a canonical key it's used on the key that registers the skin ($wgValidSkins['foobar'] = ...;), in &useskin=foobar, $wgDefaultSkin = 'foobar', the .skin-foobar css class, the skin's MediaWiki:Foobar.css (it's ucfirst-ed but it's still foobar not FooBar.css) and User:X/foobar.css, other i18n messages like skinname-foobar that defines the skin's other name and messages like foobar-action-addsection that allow the text used in tabs like "Add section" to be controlled on a per-skin basis, and currently used by every core skins and most 3rd party skins for the assets directory in $IP/skins/. * Foo Bar is a human name, this name is visible to all users on the preference page and – unlike extensions' human name – this one has full i18n support and is defined by practically every skin. This name is defined by the skinname-foobar i18n message for the foobar skin.
So:
* Extensions often end up with one name not two. * FooBar has never been exposed outside of the internal APIs for a skin (SkinFooBar class names aren't visible anywhere and if $IP/skins/FooBar.php is used it's autoloaded and not visible to anyone). * If you did expose FooBar somewhere visible one skin now has three different names for it. * And to top that off by using FooBar as the dirname a skin now basically has two different canonical names for it.
Let me detail the what I mean by multiple canonical names.
Pretend in the future we implemented a loader for extensions and skins into core, so instead of a raw require_once in your LocalSettings.php you'd drop a call to load an extension by name (maybe in this future we've moved some stuff into a .json file or something).
Something like: MWExtension::load('FooBar'); # Install $IP/extensions/FooBar
This works fine for extensions, you've got one canonical key, nothing else to deal with.
Skins using lowercase dirnames are fine too: MWSkin::load('foobar'); # Install $IP/skins/foobar/ $wgDefaultSkin = 'foobar';
The canonical keys are the same for both loading/installing skins and setting it as default.
With the introduction of FooBar "extension style" skin names that's a little different: MWSkin::load('FooBar'); # Install $IP/skins/FooBar/ $wgDefaultSkin = 'foobar';
This time two different supposedly canonical keys identifying the same skin are used for different areas of config, you'll probably end up looking them up, and anyone who tries mistakenly using `$wgDefaultSkin = 'FooBar';` is a poor soul.
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://danielfriesen.name/]
I've always found it silly that skins are installed in a different way to extensions. It seems silly to teach a user 2 ways. To be honest I still haven't got the hang of skin installation. I usually crudely plop a folder in the skins directory which I'm pretty sure is wrong.
Thus, I would personally make skins installable in the same way.
If there is a need to manage that process better, I would steer someone in the direction of writing a SkinInstaller extension. Such an extension could protect the end user from the installation process and could also provide lots of MediaWiki coolness such as reviewing skins, updating skins and publishing skins.
This would be my vision to steer towards any how...
On Wed, May 21, 2014 at 8:48 AM, Daniel Friesen daniel@nadir-seen-fire.com wrote:
On 2014-05-20, 10:28 PM, Isarra Yos wrote:
On 20/05/14 23:05, Daniel Friesen wrote:
* The skinname you require is the same one assigned to $wgValidSkins and set on $wgDefaultSkin and set on $skinname, and $stylepath. * For all skins using the old autoload pattern the assets directory remains them same, so nothing has to be re-cached.But that sounds more like an oversight with the autoloader implementation and class setup than an actual reason why it would be good practice to use lowercase. Was there a particular design reason to set those up to be that way?
I'm not sure which item, you think you're replying to but you might be misunderstanding what I'm saying.
The key used for $wgValidSkins/$wgDefaultSkin/$skinname has always been lowercase, and I haven't seen anyone suggesting that we start using &useskin=MonoBook, which would be a bad idea since this key is used in i18n message keys and the skin- css class – places you don't want to use upper case letters. Plus of course using upper case letters there is just asking for a pile of brand new bugs as someone accidentally uses the wrong case and hard to debug errors start popping up.
And for the bit on re-caching that is simple. Assets for core skins like Vector and old skins that are implemented using the autoloader are currently at: {mediawiki}/skins/vector/* If you change the directory structure so we load $IP/skins/Vector/Vector.php (instead of $IP/skin/vector/vector.php) then the path to assets becomes: {mediawiki}/skins/Vector/*
Even though not a single one of the assets have changed the URL has, so, re-cache. ResourceLoader moves most CSS, JS, and some images to load.php but there are still resources that are referred to with an actual URL, like Vector's .htc file, all css images in IE without data: URI support, css images that aren't marked with @embed (because they're too big, use multi-backgrounds, etc...), anything in an <img> using $skin->getSkinStylePath(...), and any stylesheet still loaded with addStyle (IE stylesheets, etc...).
Two of those also apply to extensions in general - FooBar and Foo Bar. Has this sort of inconsistency between name presentations ever posed a problem there? And with skins in particular, why would the internal handling of the skin name be coupled to the filename at all? And why the case-sensitivity, especially when not all platforms that mw supports even use case-sensitive filesystems? Is this just a holdover from the autoloader implementation?
Two is fine, the problem is having three, or rather in essence inventing a third just for skins.
Extensions have two names visible outside of the internal API of the extension:
- FooBar is a canonical key, it's used in the dirname (plus the entry file name) and "usually" (there is some inconsistency) the 'name' key to extension credits (.
- Foo Bar is the human name, and well, sometimes it doesn't even exist even when an extension name has multiple words.
There is some inconsistency in what we use in extension credits, we have a 'name' key and sometimes a human name gets filled in there ('PDF Handler', 'Central Auth', 'Abuse Filter', ...) and in others the canonical key gets used (ParserFunctions, SyntaxHighlight, AntiBot, TorBlock, MobileFrontend, ...), the most common behavior seems to be to just use the canonical key, giving most extensions only one name. If we were serious about human names for extensions we would make it possible to i18n them.
Skins are different, they have two names visible outside of the internal API of the skin:
- foobar is a canonical key it's used on the key that registers the skin ($wgValidSkins['foobar'] = ...;), in &useskin=foobar, $wgDefaultSkin = 'foobar', the .skin-foobar css class, the skin's MediaWiki:Foobar.css (it's ucfirst-ed but it's still foobar not FooBar.css) and User:X/foobar.css, other i18n messages like skinname-foobar that defines the skin's other name and messages like foobar-action-addsection that allow the text used in tabs like "Add section" to be controlled on a per-skin basis, and currently used by every core skins and most 3rd party skins for the assets directory in $IP/skins/.
- Foo Bar is a human name, this name is visible to all users on the preference page and – unlike extensions' human name – this one has full i18n support and is defined by practically every skin. This name is defined by the skinname-foobar i18n message for the foobar skin.
So:
- Extensions often end up with one name not two.
- FooBar has never been exposed outside of the internal APIs for a skin (SkinFooBar class names aren't visible anywhere and if $IP/skins/FooBar.php is used it's autoloaded and not visible to anyone).
- If you did expose FooBar somewhere visible one skin now has three different names for it.
- And to top that off by using FooBar as the dirname a skin now basically has two different canonical names for it.
Let me detail the what I mean by multiple canonical names.
Pretend in the future we implemented a loader for extensions and skins into core, so instead of a raw require_once in your LocalSettings.php you'd drop a call to load an extension by name (maybe in this future we've moved some stuff into a .json file or something).
Something like: MWExtension::load('FooBar'); # Install $IP/extensions/FooBar
This works fine for extensions, you've got one canonical key, nothing else to deal with.
Skins using lowercase dirnames are fine too: MWSkin::load('foobar'); # Install $IP/skins/foobar/ $wgDefaultSkin = 'foobar';
The canonical keys are the same for both loading/installing skins and setting it as default.
With the introduction of FooBar "extension style" skin names that's a little different: MWSkin::load('FooBar'); # Install $IP/skins/FooBar/ $wgDefaultSkin = 'foobar';
This time two different supposedly canonical keys identifying the same skin are used for different areas of config, you'll probably end up looking them up, and anyone who tries mistakenly using `$wgDefaultSkin = 'FooBar';` is a poor soul.
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://danielfriesen.name/]
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On 2014-05-21, 6:02 AM, Jon Robson wrote:
I've always found it silly that skins are installed in a different way to extensions. It seems silly to teach a user 2 ways. To be honest I still haven't got the hang of skin installation. I usually crudely plop a folder in the skins directory which I'm pretty sure is wrong.
Thus, I would personally make skins installable in the same way.
If there is a need to manage that process better, I would steer someone in the direction of writing a SkinInstaller extension. Such an extension could protect the end user from the installation process and could also provide lots of MediaWiki coolness such as reviewing skins, updating skins and publishing skins.
This would be my vision to steer towards any how...
The current process for installing an extension is:
1. Plop a folder in the extensions directory. 2. Put require_once( "$IP/extensions/{name}/{name}.php" ); where {name} is the name of that folder you just plopped in.
The process I'm suggesting for skins is:
1. Plop a folder in the skins directory. 2. Put require_once( "$IP/skins/{name}/{name}.php" ); where {name} is the name of that folder you just plopped in, which is also the name you will use in &useskin= or $wgDefaultSkin.
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://danielfriesen.name/]
But why create a new system to learn? What is the advantage of doing this instead of just using the tried and tested extensions directory? What stops a new user from plopping the folder into the extensions directory by accident out of habit?
On Wed, May 21, 2014 at 5:42 PM, Daniel Friesen daniel@nadir-seen-fire.com wrote:
On 2014-05-21, 6:02 AM, Jon Robson wrote:
I've always found it silly that skins are installed in a different way to extensions. It seems silly to teach a user 2 ways. To be honest I still haven't got the hang of skin installation. I usually crudely plop a folder in the skins directory which I'm pretty sure is wrong.
Thus, I would personally make skins installable in the same way.
If there is a need to manage that process better, I would steer someone in the direction of writing a SkinInstaller extension. Such an extension could protect the end user from the installation process and could also provide lots of MediaWiki coolness such as reviewing skins, updating skins and publishing skins.
This would be my vision to steer towards any how...
The current process for installing an extension is:
- Plop a folder in the extensions directory.
- Put require_once( "$IP/extensions/{name}/{name}.php" ); where {name} is the name of that folder you just plopped in.
The process I'm suggesting for skins is:
- Plop a folder in the skins directory.
- Put require_once( "$IP/skins/{name}/{name}.php" ); where {name} is the name of that folder you just plopped in, which is also the name you will use in &useskin= or $wgDefaultSkin.
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://danielfriesen.name/]
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On Wed, 21 May 2014 19:56:53 +0200, Jon Robson jdlrobson@gmail.com wrote:
What is the advantage of doing this instead of just using the tried and tested extensions directory? What stops a new user from plopping the folder into the extensions directory by accident out of habit?
Stephan has already replied to this, but I'll also note that I can't think of a single example of a software project that has both skins/themes/whatever and extensions/plugins/whatever, does not handle installing them by itself (that is, you have to copy over some files in most cases), and treats both of these types of things in the same way. I don't see why would anyone make such a mistake.
On Wed, 21 May 2014 09:48:21 +0200, Daniel Friesen daniel@nadir-seen-fire.com wrote:
And for the bit on re-caching that is simple. Assets for core skins like Vector and old skins that are implemented using the autoloader are currently at: {mediawiki}/skins/vector/* If you change the directory structure so we load $IP/skins/Vector/Vector.php (instead of $IP/skin/vector/vector.php) then the path to assets becomes: {mediawiki}/skins/Vector/* Even though not a single one of the assets have changed the URL has, so, re-cache. ResourceLoader moves most CSS, JS, and some images to load.php but there are still resources that are referred to with an actual URL, like Vector's .htc file, all css images in IE without data: URI support, css images that aren't marked with @embed (because they're too big, use multi-backgrounds, etc...), anything in an <img> using $skin->getSkinStylePath(...), and any stylesheet still loaded with addStyle (IE stylesheets, etc...).
I'm sorry, but this is very silly reasoning. Cache is not forever anyway, and ResourceLoader includes cache-busting timestamps in URL queries for each resource that goes through it (including the ones that are not embedded in CSS, and the fallbacks for data: URIs). Changing the URL is not a problem at all as long as we're not doing it every other day.
On 2014-05-24, 10:49 AM, Bartosz Dziewoński wrote:
On Wed, 21 May 2014 09:48:21 +0200, Daniel Friesen daniel@nadir-seen-fire.com wrote:
And for the bit on re-caching that is simple. Assets for core skins like Vector and old skins that are implemented using the autoloader are currently at: {mediawiki}/skins/vector/* If you change the directory structure so we load $IP/skins/Vector/Vector.php (instead of $IP/skin/vector/vector.php) then the path to assets becomes: {mediawiki}/skins/Vector/* Even though not a single one of the assets have changed the URL has, so, re-cache. ResourceLoader moves most CSS, JS, and some images to load.php but there are still resources that are referred to with an actual URL, like Vector's .htc file, all css images in IE without data: URI support, css images that aren't marked with @embed (because they're too big, use multi-backgrounds, etc...), anything in an <img> using $skin->getSkinStylePath(...), and any stylesheet still loaded with addStyle (IE stylesheets, etc...).
I'm sorry, but this is very silly reasoning. Cache is not forever anyway, and ResourceLoader includes cache-busting timestamps in URL queries for each resource that goes through it (including the ones that are not embedded in CSS, and the fallbacks for data: URIs). Changing the URL is not a problem at all as long as we're not doing it every other day.
It's not my most important point, but just a few notes since you replied.
* Just a few strict fact corrections: o The .htc doesn't get cache busted. o addStyle, etc... are cache busted by $wgStyleVersion not RL. o CSS contents in RL get cache busters, but the load.php calls themselves for skin resources don't. * The "cache" problem isn't just an issue of things unnecessarily being purged. Remember WMF's 30 day cache issues. Unless the wmfX number is already something that would be changed with two copies being simultaneously deployed the vector -> Vector path change triggers the issue of old cached pages pointing to
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://danielfriesen.name/]
On Sun, 25 May 2014 00:53:09 +0200, Daniel Friesen daniel@nadir-seen-fire.com wrote:
On 2014-05-24, 10:49 AM, Bartosz Dziewoński wrote:
I'm sorry, but this is very silly reasoning. Cache is not forever anyway, and ResourceLoader includes cache-busting timestamps in URL queries for each resource that goes through it (including the ones that are not embedded in CSS, and the fallbacks for data: URIs). Changing the URL is not a problem at all as long as we're not doing it every other day.
It's not my most important point, but just a few notes since you replied.
- Just a few strict fact corrections: o The .htc doesn't get cache busted. o addStyle, etc... are cache busted by $wgStyleVersion not RL. o CSS contents in RL get cache busters, but the load.php calls themselves for skin resources don't.
- The "cache" problem isn't just an issue of things unnecessarily being purged. Remember WMF's 30 day cache issues. Unless the wmfX number is already something that would be changed with two copies being simultaneously deployed the vector -> Vector path change triggers the issue of old cached pages pointing to
A few notes of my own, then, if you're nitpicking. :)
Yes, some resources do not include the timestamps in query. I've only mentioned the ones loaded via ResourceLoader. load.php calls would not be affected by the directory name changing and thus are not relevant.
WMF hosts each deployed version in a separate directory, changing $wgStyleDirectory (or doing some similar magic) to change the references in the pages. (Here's an example link to a resource: https://bits.wikimedia.org/static-1.24wmf5/skins/common/images/poweredby_med... – note the version number is included, this will go down in a few weeks as the versions are cycled.) This way old cached pages will never point to new version of resources and vice versa. (The old directories are kept long enough not to break anything – at least 30 days, like you said – and generally deleted in bulk every few weeks.)
On 21/05/14 07:25, Bartosz Dziewoński wrote:
- $IP/extensions/SkinName/ for everything, the rest as above. This
makes the skin work exactly like an extension. The only example I could find on mediawiki.org is the Nostalgia skin [5].
I introduced this facility to core many years ago, and I still generally think it is a good idea. There are in fact at least another 6 skins that use this system: they are in mediawiki/extensions/skins in Gerrit -- a directory layout carried over from Subversion. Nostalgia should have been put there too.
The main reason I think it is a good idea is code sharing. Treating skins as extensions means you can share automated distribution and installation systems.
-- Tim Starling
I tend to agree that skins should be installed in exactly the same way as other extensions. This will also help make installation more automateable.
-- brion On May 20, 2014 11:39 PM, "Tim Starling" tstarling@wikimedia.org wrote:
On 21/05/14 07:25, Bartosz Dziewoński wrote:
- $IP/extensions/SkinName/ for everything, the rest as above. This
makes the skin work exactly like an extension. The only example I could find on mediawiki.org is the Nostalgia skin [5].
I introduced this facility to core many years ago, and I still generally think it is a good idea. There are in fact at least another 6 skins that use this system: they are in mediawiki/extensions/skins in Gerrit -- a directory layout carried over from Subversion. Nostalgia should have been put there too.
The main reason I think it is a good idea is code sharing. Treating skins as extensions means you can share automated distribution and installation systems.
-- Tim Starling
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On 21 May 2014 13:01, Brion Vibber bvibber@wikimedia.org wrote:
I tend to agree that skins should be installed in exactly the same way as other extensions. This will also help make installation more automateable.
From a product perspective, +1 for this.
Dan
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
There seem to be three popular ways:
- $IP/skins/SkinName.php for the main file plus $IP/skins/skinname/
for
...
- $IP/skins/SkinName/ for both assets and PHP files
($IP/skins/skinname/SkinName.php etc.), using require_once in
...
- $IP/extensions/SkinName/ for everything, the rest as above. This
makes
I'd go for a mixture of option 2 and 3: $IP/skins/SkinName/SkinName.php I agree that skins and extensions should be installed in the same way as much as possible. But I do not agree that this should go so far as to install them in the same directory. They are fundamentally different things - one concerns (ideally) the functionality of the wiki, the other (ideally) its presentation. "But it can both be pulled in using require_once, let's put it in the same place." does not sound right to me. Using that reasoning you could just put all of MW in one directory (or in one file, even) and be done with it. At the same time I do not agree that the skin absolutely has be called the same in all contexts so admins do not get too confused. That's what installation instructions are for. "Plop it into yaddayadda. Done. Activate it using blahdiblah. Done." And, incidentally, what's wrong with &useskin=MonoBook?
Btw: "What stops a new user from plopping the folder into the extensions directory by accident out of habit?"
Three answers - 1 serious, 1 half-serious, 1 not helpful at all (you can pick which is which): 1. "New user", "out of habit"? Umm... 2. Nothing. And any decently written skin should not care. 3. Composer. :P And a decent extension/skin manager built on top of that. (But that's a completely different discussion, agreed.)
Cheers, Stephan
On Wed, May 21, 2014 at 1:38 PM, Stephan Gambke s7eph4n@gmail.com wrote:
Using that reasoning you could just put all of MW in one directory (or in one file, even) and be done with it.
That's the most sensible idea I've read all week :D
-Chad
On 21/05/14 20:38, Stephan Gambke wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
There seem to be three popular ways:
- $IP/skins/SkinName.php for the main file plus $IP/skins/skinname/
for
...
- $IP/skins/SkinName/ for both assets and PHP files
($IP/skins/skinname/SkinName.php etc.), using require_once in
...
- $IP/extensions/SkinName/ for everything, the rest as above. This
makes
I'd go for a mixture of option 2 and 3: $IP/skins/SkinName/SkinName.php I agree that skins and extensions should be installed in the same way as much as possible. But I do not agree that this should go so far as to install them in the same directory. They are fundamentally different things - one concerns (ideally) the functionality of the wiki, the other (ideally) its presentation. "But it can both be pulled in using require_once, let's put it in the same place." does not sound right to me. Using that reasoning you could just put all of MW in one directory (or in one file, even) and be done with it. At the same time I do not agree that the skin absolutely has be called the same in all contexts so admins do not get too confused. That's what installation instructions are for. "Plop it into yaddayadda. Done. Activate it using blahdiblah. Done." And, incidentally, what's wrong with &useskin=MonoBook?
Btw: "What stops a new user from plopping the folder into the extensions directory by accident out of habit?"
Three answers - 1 serious, 1 half-serious, 1 not helpful at all (you can pick which is which):
- "New user", "out of habit"? Umm...
- Nothing. And any decently written skin should not care.
- Composer. :P And a decent extension/skin manager built on top of that. (But that's a completely different discussion, agreed.)
Cheers, Stephan
This. All of this.
"And, incidentally, what's wrong with &useskin=MonoBook?"
I think the only thing really wrong with it is that support for it hasn't been implemented; the expected use on the user end is indeed the that, the normal capitalisation, and I've had a few regular users on my talkpage confused about it in the past, especially since everything else in the url string tends to be case-insensitive or may even automatically convert lowercase to camelcase in other instances.
If the other internals all expect lowercase, internal functions should be more than capable of automatically converting the name for their use. This, however, has also not been implemented in core; as is most skin extensions just have an extra line in the definition file to convert it for those.
Also some more on why putting skins in the extensions directory is a bad idea:
From a sysadmin standpoint, the extensions repository/directory is basically a horrible pit of extreme darkness full of many things that that you basically have no idea what they are because their names are completely unhelpful. This is kind of not good, and we do not want to make it worse, because unlike extension names, which sometimes are helpful, skin names basically NEVER give the slightest indication what they are. Skin names are generally completely random things that sounded good, or seemed like a good idea at the time, or were made up as drunken placeholders that nobody got around to replacing. When they even resemble the appearance of the skin itself, it is often a complete coincidence (with a some exceptions - for instance I named a skin 'greystuff' once and indeed it is basically a bunch of grey stuff, but that was only because I did remember replace the original drunken placeholder).
Whereas if we put them somewhere else, such as in 'skins', a place where people would probably expect to find skins, it is immediately clear what they are, both in the filesystem and in the settings files. This is very nice, and leads to fewer incidents of fellow sysadmins killing themselves with shovels.
-I
On Wed, 21 May 2014 22:38:00 +0200, Stephan Gambke s7eph4n@gmail.com wrote:
There seem to be three popular ways:
- $IP/skins/SkinName.php for the main file plus $IP/skins/skinname/
for
...
- $IP/skins/SkinName/ for both assets and PHP files
($IP/skins/skinname/SkinName.php etc.), using require_once in
...
- $IP/extensions/SkinName/ for everything, the rest as above. This
makes
I'd go for a mixture of option 2 and 3: $IP/skins/SkinName/SkinName.php
Yes, this is what I meant; that was just a typo, sorry.
Thank you for all the comments! I've had a few busy days, sorry for only replying now. I'm going to try summarizing everything.
I have replied to a few tangenial messages separately. Many things have been said here (and in several disconnected threads), so I might have missed something – please remind us if you think I did :)
Table of contents: * Skin autodiscovery * Capitalization of skin names and their file/directory names * On 'skins/' vs 'extensions/' * Handling WMF deployment
----
First of all, something we all agree on: let's murder skin autodiscovery. I'll submit patches to emit deprecation warnings if a skin using it is found (to master and 1.23 LTS release), and another patch that will remove it entirely (intended for MW 1.24 or MW 1.25).
I will also convert the core skins not to use autodiscovery first. I'll keep them in 'skins/' for now, it'll be less disruptive in WMF environment and easy to change later if we decide to.
----
As for Daniel's concerns about capitalization differences (which are valid regardless of which directory we want to place the skins in), forcing everything to be lowercase is a solution, but one that I don't like very much. Keeping the status quo is also a solution, but clearly no one at all likes this one.
I'm going to name the three names we have 'skin file name' (same as skin directory name), 'skin name' (the one used in $wgValidSkins, saved in user preferences etc., used for site and user JS and CSS pages) and 'proper skin name' (localised one displayed on Special:Preferences).
Let me propose two more ways we could do this. (All of the back-compat concerns below don't apply to new skins, only existing ones that we'd want to retro-fit to the new way. We could also avoid them by not retro-fitting.)
* Instead of switching file name to lowercase, why not switch skin name to CamelCase? We'd have a maintenance script to update user preferences in the database, and a little back-compat layer to allow skins to define non-canonical names they want to respond to (for site admins who can't run the maintenance script).
This will, however, break back-compat for JS user scripts (that implement different behavior per-skin), as well as very poorly written PHP-side code (which should have no reason to care about skins). This might be acceptable.
* Always compare skin names case-insensitively (which is easy if they're all-ASCII), in 'useskin', in database user preferences data, everywhere – and use CamelCased names in "user-visible" areas. We could keep the canonical skin names the way they are in existing skins for back-compat of user preferences, but site admins would be able to set $wgDefaultSkin = 'CologneBlue' even if the canonical skin name was 'cologneblue'.
As far as I can tell this will not break back-compat at all, and would let us move forward leaving very little tech debt behind.
* (Or maybe sidestep the issue, say that you can do this in whichever way and just keep the current way in existing skins – this would be lowercase everywhere for most, if not all. This would suck though. :) )
----
As for where to put the skins, which is what most people care about.
Reading the replies, most people who actually make skins seem to prefer the 'skins/' directory, and have provided some excellent reasoning for this (better than mine, thanks people!). I've been undecided myself, but right now I really feel this is the way we should go.
Excerpts:
On Tue, 20 May 2014 23:38:14 +0200, Tyler Romeo tylerromeo@gmail.com wrote:
I do agree with keeping skins in a separate directory than extensions. It implies a bit of control on our part concerning how skins need to be structured, whereas if they were extensions, we cannot place any requirements on the structure.
On Wed, 21 May 2014 00:06:57 +0200, Jack Phoenix jack@countervandalism.net wrote:
CamelCase is how we name things consisting of multiple words (i.e. BlueSky, DuskToDawn, HowTo, ...), so it's only reasonable to use CamelCase here too.
On Wed, 21 May 2014 20:10:40 +0200, Nick White nick.white@durham.ac.uk wrote:
the skins/ directory is a sensible place for skins, and the fact that they in practise aren't fundamentally different to extensions is largely irrelevant.
On Wed, 21 May 2014 22:38:00 +0200, Stephan Gambke s7eph4n@gmail.com wrote:
I agree that skins and extensions should be installed in the same way as much as possible. But I do not agree that this should go so far as to install them in the same directory. They are fundamentally different things - one concerns (ideally) the functionality of the wiki, the other (ideally) its presentation.
On Thu, 22 May 2014 01:31:57 +0200, Isarra Yos zhorishna@gmail.com wrote:
From a sysadmin standpoint, the extensions repository/directory is basically a horrible pit of extreme darkness full of many things that that you basically have no idea what they are because their names are completely unhelpful. This is kind of not good, and we do not want to make it worse, because unlike extension names, which sometimes are helpful, skin names basically NEVER give the slightest indication what they are. (…)
Whereas if we put them somewhere else, such as in 'skins', a place where people would probably expect to find skins, it is immediately clear what they are, both in the filesystem and in the settings files. (…)
----
However, WMF ops/core/platform people seem to prefer 'extensions/', because we've already worked out the process for extensions so it'd be nice not to have to do anything else for skins ;).
Given the arguments above and overwhelming support of people who actually do this stuff, I think that putting skins in 'extensions/' should really be a last-resort solution if everything else fails.
Are there any fundamental, insurmountable issues with having skins in 'skins/' in WMF production? Or is it just a matter of performing some boring, but simple changes to deployment scripts and processes (wrapping things in `foreach ( ['skins', 'extensions'] as $dir )` etc.)? If it's just that, I'd be more than willing to help applying them (I generally have some idea on how this stuff works and where to find it, but I'd probably need help and of course code reviews).
So, I'm proposing an experiment: We can evaluate this almost painlessly by converting the current Nostalgia extension-skin to a skin in my proposed format, deploying it on nostalgia.wikimedia.org (after doing whatever changes are necessary to deployment things first) and seeing what happens. This should not affect the rest of the cluster, and even catastrophic failures would have a very tiny "surface area". If it turns out that this is really not going to work, we can always revert all changes and stick with stuffing skins in 'extensions/' instead.
Tim, Brion, Dan: How does this look to you?
----
Thank you for all the comments again!
On 2014-05-24, 12:12 PM, Bartosz Dziewoński wrote:
First of all, something we all agree on: let's murder skin autodiscovery. I'll submit patches to emit deprecation warnings if a skin using it is found (to master and 1.23 LTS release), and another patch that will remove it entirely (intended for MW 1.24 or MW 1.25).
+1
As for Daniel's concerns about capitalization differences (which are valid regardless of which directory we want to place the skins in), forcing everything to be lowercase is a solution, but one that I don't like very much. Keeping the status quo is also a solution, but clearly no one at all likes this one.
I'm going to name the three names we have 'skin file name' (same as skin directory name), 'skin name' (the one used in $wgValidSkins, saved in user preferences etc., used for site and user JS and CSS pages) and 'proper skin name' (localised one displayed on Special:Preferences).
Let me propose two more ways we could do this. (All of the back-compat concerns below don't apply to new skins, only existing ones that we'd want to retro-fit to the new way. We could also avoid them by not retro-fitting.)
- Instead of switching file name to lowercase, why not switch skin
name to CamelCase? We'd have a maintenance script to update user preferences in the database, and a little back-compat layer to allow skins to define non-canonical names they want to respond to (for site admins who can't run the maintenance script).
This will, however, break back-compat for JS user scripts (that implement different behavior per-skin), as well as very poorly written PHP-side code (which should have no reason to care about skins). This might be acceptable.
- Always compare skin names case-insensitively (which is easy if they're all-ASCII), in 'useskin', in database user preferences data,
everywhere – and use CamelCased names in "user-visible" areas. We could keep the canonical skin names the way they are in existing skins for back-compat of user preferences, but site admins would be able to set $wgDefaultSkin = 'CologneBlue' even if the canonical skin name was 'cologneblue'.
As far as I can tell this will not break back-compat at all, and would let us move forward leaving very little tech debt behind.
The idea of "migrating" a pile of keys in the first makes me shudder.
In both of these cases please keep some other areas in mind: * i18n message keys like skinname-{$skinname} and {$skinname}-action-viewsource (which the i18n system as set a standard of always using lower case keys) * Site skin scripts and styles MediaWiki:{ucfirst($skinname)}.css + MediaWiki:{ucfirst($skinname)}.js and user scripts and styles User:X/{$skinname}.css + MediaWiki:{ucfirst($skinname)}.js (these would be ridiculously hard to just "migrate".
So either way even if you try replacing skinname with SkinName, the lower case skinname still sticks around.
I am fine with the idea of letting &useskin= and $wgDefaultSkin (and even $wgSkipSkins) work case insensitively (though I wouldn't encourage the actual use of uppercase there). However $wgValidSkins isn't something should become case insensitive, attempting that for array keys is asking for bugs. Same for putting non lowercase keys in the database and trying to make them case insensitive. The easiest way to make &useskin=, $wgDefaultSkin, and $wgSkipSkins case insensitive is to normalize all skin keys from them to lowercase (which is actually only a few lines in Skin.php) and then as a "breaking change" say we're forbidding non-lowercase keys to $wgValidSkins (which should be rather simple since I haven't seen a single skin yet doing that which would actually be broken by such a change).
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://danielfriesen.name/]
On Sun, 25 May 2014 00:11:28 +0200, Daniel Friesen daniel@nadir-seen-fire.com wrote:
However $wgValidSkins isn't something should become case insensitive, attempting that for array keys is asking for bugs. Same for putting non lowercase keys in the database and trying to make them case insensitive. The easiest way to make &useskin=, $wgDefaultSkin, and $wgSkipSkins case insensitive is to normalize all skin keys from them to lowercase (which is actually only a few lines in Skin.php) and then as a "breaking change" say we're forbidding non-lowercase keys to $wgValidSkins (which should be rather simple since I haven't seen a single skin yet doing that which would actually be broken by such a change).
Hmm. Yeah, this actually sounds very sensible. Let's make it so.
To summarize:
* 'skin file name' (='skin directory name' and 'skin repository name'): * "pretty" (that is, usually CamelCased, unless the skin name would for some reason be lowercase) * may not contain non-ASCII characters * 'skin name': * a lowercase version of 'skin file name', which would also provide any future skin loading/installation/management mechanism with a simple way to map the file/directory/repository name to the 'skin name' * user inputs (useskin, $wgDefaultSkin) are accepted with any capitalization and normalized to lowercase
The requirements above are technically breaking changes, but are very unlikely to actually break anything.
Right?
A skin has (or imho should have) (only) two "name" like properties:
1) Internal identifier ("id"): - Used for processing at the application level and as public API (configuration variables, url parameters, API properties, database values, dynamically constructed page names such as for site and user scripts, preference keys) - Lowercase - No non-ASCII characters - No spaces - Not localised - Change this would be a breaking change
2) Display title ("skin name"): - Typically starts with an uppercase letter, may contain spaces (e.g. "Cologne Blue") - Used for graphical user interface (e.g. anywhere HTML is displayed, whenever it is used inside an message) - Defined by msg key skinnname-{skin}
— Krinkle
On 25 May 2014, at 18:02, Bartosz Dziewoński matma.rex@gmail.com wrote:
On Sun, 25 May 2014 00:11:28 +0200, Daniel Friesen daniel@nadir-seen-fire.com wrote:
However $wgValidSkins isn't something should become case insensitive, attempting that for array keys is asking for bugs. Same for putting non lowercase keys in the database and trying to make them case insensitive. The easiest way to make &useskin=, $wgDefaultSkin, and $wgSkipSkins case insensitive is to normalize all skin keys from them to lowercase (which is actually only a few lines in Skin.php) and then as a "breaking change" say we're forbidding non-lowercase keys to $wgValidSkins (which should be rather simple since I haven't seen a single skin yet doing that which would actually be broken by such a change).
Hmm. Yeah, this actually sounds very sensible. Let's make it so.
To summarize:
- 'skin file name' (='skin directory name' and 'skin repository name'):
- "pretty" (that is, usually CamelCased, unless the skin name would for some reason be lowercase)
- may not contain non-ASCII characters
- 'skin name':
- a lowercase version of 'skin file name', which would also provide any future skin loading/installation/management mechanism with a simple way to map the file/directory/repository name to the 'skin name'
- user inputs (useskin, $wgDefaultSkin) are accepted with any capitalization and normalized to lowercase
The requirements above are technically breaking changes, but are very unlikely to actually break anything.
Right?
-- Matma Rex
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On Sat, 24 May 2014 21:12:26 +0200, Bartosz Dziewoński matma.rex@gmail.com wrote:
First of all, something we all agree on: let's murder skin autodiscovery. I'll submit patches to emit deprecation warnings if a skin using it is found (to master and 1.23 LTS release), and another patch that will remove it entirely (intended for MW 1.24 or MW 1.25). I will also convert the core skins not to use autodiscovery first. I'll keep them in 'skins/' for now, it'll be less disruptive in WMF environment and easy to change later if we decide to.
I filed a bug to track this: https://bugzilla.wikimedia.org/show_bug.cgi?id=65748
And submitted patches that seem to do a complete job:
https://gerrit.wikimedia.org/r/#/c/135383/ (master) "Don't use autodiscovery for core skins" https://gerrit.wikimedia.org/r/#/c/135384/ (master) "Move core skins to separate directories" https://gerrit.wikimedia.org/r/#/c/135429/ (master) "Officially deprecate skin autodiscovery" https://gerrit.wikimedia.org/r/#/c/135427/ (REL1_23) "Officially deprecate skin autodiscovery" https://gerrit.wikimedia.org/r/#/c/135439/ (master, on hold) "Completely remove skin autodiscovery"
Reviews welcome. Especially the REL1_23 is a bit urgent, as the release is around the corner.
I will create a brief migration guide for creators and users of custom skins later this week (or next week), probably at https://www.mediawiki.org/wiki/Manual:Skin_autodiscovery.
On 24 May 2014 14:12, Bartosz Dziewoński matma.rex@gmail.com wrote:
Are there any fundamental, insurmountable issues with having skins in 'skins/' in WMF production?
I am not a developer and therefore I cannot speak for them, but from a product perspective I have no insurmountable issues with either the /skins/ or the /extensions/ solution. It seems that there's pros and cons for each solution, so as long as it's handled appropriately (and it certainly seems to me like you're doing that), then you've got my support.
So, I'm proposing an experiment: We can evaluate this almost painlessly by converting the current Nostalgia extension-skin to a skin in my proposed format, deploying it on nostalgia.wikimedia.org (after doing whatever changes are necessary to deployment things first) and seeing what happens. This should not affect the rest of the cluster, and even catastrophic failures would have a very tiny "surface area". If it turns out that this is really not going to work, we can always revert all changes and stick with stuffing skins in 'extensions/' instead.
Tim, Brion, Dan: How does this look to you?
Well, you've got my endorsement! How can I help you make this happen?
Thanks, Dan
On Tue, 27 May 2014 05:28:41 +0200, Dan Garry dgarry@wikimedia.org wrote:
I am not a developer and therefore I cannot speak for them, but from a product perspective I have no insurmountable issues with either the /skins/ or the /extensions/ solution. It seems that there's pros and cons for each solution, so as long as it's handled appropriately (and it certainly seems to me like you're doing that), then you've got my support.
So, I'm proposing an experiment: We can evaluate this almost painlessly by converting the current Nostalgia extension-skin to a skin in my proposed format, deploying it on nostalgia.wikimedia.org (after doing whatever changes are necessary to deployment things first) and seeing what happens.
Well, you've got my endorsement! How can I help you make this happen?
It just so happened that Chad has recently merged a parallel set of changes that moved two skins out of core – Modern and Cologne Blue (https://gerrit.wikimedia.org/r/#/c/118345/ and linked patches) – and today he submitted patches that would make it possible to deploy the two new repositories in the skins/ directory. (Thanks Chad!)
See https://bugzilla.wikimedia.org/show_bug.cgi?id=65868 about the progress.
This is going to roll out with tomorrow's deployment and should be unnoticeable for users.
On 2014-05-20, 2:25 PM, Bartosz Dziewoński wrote:
- $IP/skins/SkinName/ for both assets and PHP files
($IP/skins/skinname/SkinName.php etc.), using require_once in LocalSettings like extensions to load the skin, manually adding an entry to $wgValidSkinNames in the main PHP file. This seems to be the preferred method among "modern" skins, for example Erudite [3] or Nimbus [4].
We've been discussing $IP/skins/skinname/skinname.php vs. $IP/skins/SkinName/SkinName.php for awhile, but it occurs to me now – after reading this patchset[1] – that it's possible some people in the discussion may actually not know or have a misunderstanding of what this file is.
$IP/skins/vector/vector.php or $IP/skins/Vector/Vector.php (whichever we go with) is NOT what is currently at $IP/skins/Vector.php.
$IP/skins/Vector.php (and the like) contain classes like SkinVector and VectorTemplate.
The proposed $IP/skins/vector/vector.php and $IP/skins/Vector/Vector.php are meta(?) files – like an extension's $IP/extensions/{name}/{name}.php file – which are responsible for setting things like the skin's credits, autoloading, i18n, resourceloader modules, and registering the skin inside $wgValidSkinNames.
For example my tutorial[1] describes 3 files in the skin structure:
* skins/myskin/myskin.php – The new skin file containing definitions for credits, $wgValidSkinNames, $wgAutoloadClasses, $ wgExtensionMessagesFiles, and $wgResourceModules. * skins/myskin/MySkin.skin.php – The file containing SkinMySkin and MySkinTemplate classes. The naming came from the FooBar.hooks.php pattern some extensions use but can be anything you want. * skins/myskin/MySkin.i18n.php – The old i18n for the skin (from the pre-json era of i18n). Likewise the name can be anything you want.
[1] https://gerrit.wikimedia.org/r/#/c/135384/ [2] https://www.mediawiki.org/wiki/Manual:Skinning/Tutorial#Creating_the_skin
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://danielfriesen.name/]
I'm not happy at all with how the skin naming topic "ended".
Throughout the discussion I explained a variety of reasons why switching to a CamelCased structure and skin name was a bad idea. We discussed them, some like the caching issue got valid comments back on why they're not issues, others remained. But while there was a lot of discussion on the points on why the change was a bad idea, almost no one seems to have bothered detailing points why it's a good idea and comparing them to the negative point.
Reading through the discussion, the only person who bothered making a single point on why it was a good idea was Jack Phoenix:
I'm going to assume that the lowercase "skinname" in $IP/skins/skinname/SkinName.php is just a typo and you meant it to be CamelCased as "SkinName". If and when so, yes, this is what should be our recommended way of doing it. CamelCase is how we name things consisting of multiple words (i.e. BlueSky, DuskToDawn, HowTo, ...), so it's only reasonable to use CamelCase here too. Having written and cleaned up many skins myself, this is the naming convention I prefer and that seems natural right from the start.
For why it's a bad idea, I made various other points, some of the remaining ones summed up:
* Unlike extension names, skin names are used in a variety of locations which upper case letters are either considered bad or problematic, such as i18n keys, and keys to $wgValidSkinNames which is both used in those i18n keys and has to be lower cased if we want to make &useskin= case insensitive. So the lower cased keys will never actually go away. * ((Not a point in favour, but when someone pointed out that extensions already have 3 names I pointed out I explained that in most cases they actually only have 1, at most 2)) * Skins only expose 2 names to site admins and users, the canonical lower case key and a fully translatable key. Because the lower case key doesn't actually disappear exposing a CamelCase name now gives skins 3 names to deal with – more than people installing extensions deal with – and makes 2 of those names "canonical" names that are supposed to uniquely identify the skin outside of the internals.
((Btw, I'll also point out that if we ever managed to implement real class-less skins using templates our current only need for CamelCase names – skin class names – would actually disappear))
As the discussion and attempts to implement this move on: Krinkle chimes in that skins should only have a lowercase non-ASCII id and a localized skin name. I help out reviewing the various patchsets trying to implement this, giving normal reviews, and also giving tips on how to properly write a skin that uses an upper case directory (because most people here trying it don't know how to do so without doing things like completely breaking the IE compatibility features in a skin). Bartosz agrees to leave skin directory names as-is while moving stuff out of core, at least while the discussion hasn't finished.
Then all of a sudden, while the discussion is still open, it's reported that someone who isn't even a maintainer for skin related stuff +2s something that changes skinname -> SkinName directories for some core skins, and documentation starts getting changed to say that skins use CamelCased directory names.
As unhappy as I am with my job revolving around themes and skins, I'm probably currently the one with the most knowledge on how the internals of our skinning system work. And it feels like me and one of the +2 maintainers for this skinning stuff in core are being ignored.
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://danielfriesen.name/]
Some ideas I AM currently in favour of:
* Deprecate and drop the autoloader. * Split the classes in skin files into separate class files. * Move skins out of core (but I still believe our lower case skin directory names should remain lower case) * Make the user facing &useskin=, etc... case insensitive by normalizing it to the canonical lower case string in $wgValidSkinNames * Introduce 'namemsg' so we can deprecate 'name' like we did 'description' and replace our half-assed and inconsistent English extension/skin names – some with spaces others just CamelCased words – with true localized names for both extensions and skins. * Make it possible for RL to use conditional IE stylesheets instead of requiring addStyle for that. * Make it possible for RL or something else to use assets like those we'd use getSkinStylePath for without needing $stylepath (bonus points if you implement this in a way that eliminates the child skin issues with $skinname and $stylepath).
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://danielfriesen.name/]
I'll not answer all your points. I'd just like to (again) question the need to keep all the names and identifiers of a skin identical whatever the cost. Maybe I am daft, everybody else seems to take this as some given great goal and nobody else is questioning it. As I see it it introduces all kinds of dependencies for no better reason than to not confuse some hypothetical user or admin. Decoupling has apparently gone out of fashion. And it is biting you already, you have or will have to special case and transform identifiers to somehow make it work introducing loads of bugs along the way. It adds more bad design to an already badly flawed skin system.
I18n message names that are directly derived from some value in $wgValidSkinNames? What?
And what's the value of lower-casing the useskin param? If some skin developer thought it was a great idea to use camel case there or upper case or whatever, why do you care? They will get fire from their users soon enough. I think it is a bad idea to convolute core code with special transformations just to protect the users from some skin designers bad decisions. It makes the core code slow and unmaintainable. And in the end it introduces exactly the kind of imprecision into a skin's specs that we are trying to avoid. Because now with all combinations of uc and lc being valid we really have created a case where nobody knows exactly what is supposed to go into this useskin param and why.
Stephan
On 1 June 2014 09:44, Daniel Friesen daniel@nadir-seen-fire.com wrote:
I'm not happy at all with how the skin naming topic "ended".
Throughout the discussion I explained a variety of reasons why switching to a CamelCased structure and skin name was a bad idea. We discussed them, some like the caching issue got valid comments back on why they're not issues, others remained. But while there was a lot of discussion on the points on why the change was a bad idea, almost no one seems to have bothered detailing points why it's a good idea and comparing them to the negative point.
Reading through the discussion, the only person who bothered making a single point on why it was a good idea was Jack Phoenix:
I'm going to assume that the lowercase "skinname" in $IP/skins/skinname/SkinName.php is just a typo and you meant it to be CamelCased as "SkinName". If and when so, yes, this is what should be our recommended way of doing it. CamelCase is how we name things consisting of multiple words (i.e. BlueSky, DuskToDawn, HowTo, ...), so it's only reasonable to use CamelCase here too. Having written and cleaned up many skins myself, this is the naming convention I prefer and that seems natural right from the start.
For why it's a bad idea, I made various other points, some of the remaining ones summed up:
- Unlike extension names, skin names are used in a variety of locations which upper case letters are either considered bad or problematic, such as i18n keys, and keys to $wgValidSkinNames which is both used in those i18n keys and has to be lower cased if we want to make &useskin= case insensitive. So the lower cased keys will never actually go away.
- ((Not a point in favour, but when someone pointed out that extensions already have 3 names I pointed out I explained that in most cases they actually only have 1, at most 2))
- Skins only expose 2 names to site admins and users, the canonical lower case key and a fully translatable key. Because the lower case key doesn't actually disappear exposing a CamelCase name now gives skins 3 names to deal with – more than people installing extensions deal with – and makes 2 of those names "canonical" names that are supposed to uniquely identify the skin outside of the internals.
((Btw, I'll also point out that if we ever managed to implement real class-less skins using templates our current only need for CamelCase names – skin class names – would actually disappear))
As the discussion and attempts to implement this move on: Krinkle chimes in that skins should only have a lowercase non-ASCII id and a localized skin name. I help out reviewing the various patchsets trying to implement this, giving normal reviews, and also giving tips on how to properly write a skin that uses an upper case directory (because most people here trying it don't know how to do so without doing things like completely breaking the IE compatibility features in a skin). Bartosz agrees to leave skin directory names as-is while moving stuff out of core, at least while the discussion hasn't finished.
Then all of a sudden, while the discussion is still open, it's reported that someone who isn't even a maintainer for skin related stuff +2s something that changes skinname -> SkinName directories for some core skins, and documentation starts getting changed to say that skins use CamelCased directory names.
As unhappy as I am with my job revolving around themes and skins, I'm probably currently the one with the most knowledge on how the internals of our skinning system work. And it feels like me and one of the +2 maintainers for this skinning stuff in core are being ignored.
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://danielfriesen.name/]
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On 2014-06-01, 3:25 AM, Stephan Gambke wrote:
I'll not answer all your points. I'd just like to (again) question the need to keep all the names and identifiers of a skin identical whatever the cost. Maybe I am daft, everybody else seems to take this as some given great goal and nobody else is questioning it. As I see it it introduces all kinds of dependencies for no better reason than to not confuse some hypothetical user or admin. Decoupling has apparently gone out of fashion. And it is biting you already, you have or will have to special case and transform identifiers to somehow make it work introducing loads of bugs along the way. It adds more bad design to an already badly flawed skin system.
What kind of decoupling did you have in mind? This is way to abstract/ambiguous to write a response to.
I18n message names that are directly derived from some value in $wgValidSkinNames? What?
Yes, specifically the key.
When you set $wgValidSkinName for a skin: $wgValidSkinName['foobar'] = 'FooBar';
The value 'FooBar' is used to determine the SkinTemplate subclass to use, in this case SkinFooBar. (I once tried to make it possible to explicitly specify the class name instead of relying on the ancient "Prepend with 'Skin' behaviour" but that hasn't been merged) And the key 'foobar' is used: - In Skin::getSkinNameMessages to find the i18n key (in this case skinname-foobar) with the skin's name.
The $skinname you place in SkinFooBar (which is supposed to match the key in $wgValidSkinNames) is used: - In OutputPage::makeResourceLoaderLink to pass skin= to RL modules - Which is then used by ResourceLoaderSiteModule::getPages to determine the MediaWiki:{ucfirst($skinname)}.{js,css} to use. - Giving us MediaWiki:Foobar.css and MediaWiki:Foobar.js
And what's the value of lower-casing the useskin param? If some skin developer thought it was a great idea to use camel case there or upper case or whatever, why do you care? They will get fire from their users soon enough. I think it is a bad idea to convolute core code with special transformations just to protect the users from some skin designers bad decisions. It makes the core code slow and unmaintainable. And in the end it introduces exactly the kind of imprecision into a skin's specs that we are trying to avoid. Because now with all combinations of uc and lc being valid we really have created a case where nobody knows exactly what is supposed to go into this useskin param and why.
&useskin= is a user facing interface. I can't remember where in the discussion someone pointed out that – while most users user &useskin=monobook – some users have tried using &useskin=MonoBook and come asking for help. So the topic of making it case insensitive came up, then both work.
However the normal way of implementing case insensitive comparison is simply to compare two lower case strings. However in this situation &useskin= is not compared but rather used as an array key. Essentially (not exactly, but in summary similar to): $wgValidSkinNames[$useskin]
The normal way one would make $useskin in this situation case insensitive wold be, essentially: $wgValidSkinNames[strtolower($useskin)]
I was pointing out that – besides various other side effects – someone can't suggest we start doing '$wgValidSkinNames['FooBar'] = 'FooBar';` to eliminate the lowercase 'foobar' in favour of 'FooBar' if we want to make &useskin= case insensitive because we can't case insensitively get `$wgValidSkinNames['FooBar']` from a $useskin that is 'foobar' or 'FOOBAR'.
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://danielfriesen.name/]
On 1 June 2014 22:45, Daniel Friesen daniel@nadir-seen-fire.com wrote:
What kind of decoupling did you have in mind?
Not specifying that each skin has to have exactly one lc identifier and then starting to rely on this requirement and generate all sorts of secondary names, identifiers, paths, class names, etc. from that. E.g why not just ask that skin for it's localized name?
I know there is loads of legacy code to deal with here and this business with the message identifiers for the skin names in particular is not the object of the on-going changes. It's just that I'd rather not have an explicit requirement introduced specifying that there must be exactly one all-purpose lower-case id per skin.
What kind of decoupling did you have in mind?
Not specifying that each skin has to have exactly one lc identifier and then starting to rely on this requirement and generate all sorts of secondary names, identifiers, paths, class names, etc. from that. E.g why not just ask that skin for it's localized name?
I second this, code (skin or extension) should be expressive and if possible be decoupled. Doing all sorts of magic behind a curtain may save some line of code but it certainly does not improve readability or expressiveness and makes it prone to breakage if some of the "magic" disappears.
On 6/2/14, Stephan Gambke s7eph4n@gmail.com wrote:
On 1 June 2014 22:45, Daniel Friesen daniel@nadir-seen-fire.com wrote:
What kind of decoupling did you have in mind?
Not specifying that each skin has to have exactly one lc identifier and then starting to rely on this requirement and generate all sorts of secondary names, identifiers, paths, class names, etc. from that. E.g why not just ask that skin for it's localized name?
I know there is loads of legacy code to deal with here and this business with the message identifiers for the skin names in particular is not the object of the on-going changes. It's just that I'd rather not have an explicit requirement introduced specifying that there must be exactly one all-purpose lower-case id per skin.
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Hey. May I please know how to unsubscribe from the mails from Wikimedia?? On 1 Jun 2014 13:14, "Daniel Friesen" daniel@nadir-seen-fire.com wrote:
I'm not happy at all with how the skin naming topic "ended".
Throughout the discussion I explained a variety of reasons why switching to a CamelCased structure and skin name was a bad idea. We discussed them, some like the caching issue got valid comments back on why they're not issues, others remained. But while there was a lot of discussion on the points on why the change was a bad idea, almost no one seems to have bothered detailing points why it's a good idea and comparing them to the negative point.
Reading through the discussion, the only person who bothered making a single point on why it was a good idea was Jack Phoenix:
I'm going to assume that the lowercase "skinname" in $IP/skins/skinname/SkinName.php is just a typo and you meant it to be CamelCased as "SkinName". If and when so, yes, this is what should be our recommended way of doing it. CamelCase is how we name things consisting
of
multiple words (i.e. BlueSky, DuskToDawn, HowTo, ...), so it's only reasonable to use CamelCase here too. Having written and cleaned up many skins myself, this is the naming convention I prefer and that seems
natural
right from the start.
For why it's a bad idea, I made various other points, some of the remaining ones summed up:
- Unlike extension names, skin names are used in a variety of locations which upper case letters are either considered bad or problematic, such as i18n keys, and keys to $wgValidSkinNames which is both used in those i18n keys and has to be lower cased if we want to make &useskin= case insensitive. So the lower cased keys will never actually go away.
- ((Not a point in favour, but when someone pointed out that extensions already have 3 names I pointed out I explained that in most cases they actually only have 1, at most 2))
- Skins only expose 2 names to site admins and users, the canonical lower case key and a fully translatable key. Because the lower case key doesn't actually disappear exposing a CamelCase name now gives skins 3 names to deal with – more than people installing extensions deal with – and makes 2 of those names "canonical" names that are supposed to uniquely identify the skin outside of the internals.
((Btw, I'll also point out that if we ever managed to implement real class-less skins using templates our current only need for CamelCase names – skin class names – would actually disappear))
As the discussion and attempts to implement this move on: Krinkle chimes in that skins should only have a lowercase non-ASCII id and a localized skin name. I help out reviewing the various patchsets trying to implement this, giving normal reviews, and also giving tips on how to properly write a skin that uses an upper case directory (because most people here trying it don't know how to do so without doing things like completely breaking the IE compatibility features in a skin). Bartosz agrees to leave skin directory names as-is while moving stuff out of core, at least while the discussion hasn't finished.
Then all of a sudden, while the discussion is still open, it's reported that someone who isn't even a maintainer for skin related stuff +2s something that changes skinname -> SkinName directories for some core skins, and documentation starts getting changed to say that skins use CamelCased directory names.
As unhappy as I am with my job revolving around themes and skins, I'm probably currently the one with the most knowledge on how the internals of our skinning system work. And it feels like me and one of the +2 maintainers for this skinning stuff in core are being ignored.
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://danielfriesen.name/]
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
To unsubscribe go here: https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On 6/1/14, Sajjad Manal sajjadmanal24@gmail.com wrote:
Hey. May I please know how to unsubscribe from the mails from Wikimedia?? On 1 Jun 2014 13:14, "Daniel Friesen" daniel@nadir-seen-fire.com wrote:
I'm not happy at all with how the skin naming topic "ended".
Throughout the discussion I explained a variety of reasons why switching to a CamelCased structure and skin name was a bad idea. We discussed them, some like the caching issue got valid comments back on why they're not issues, others remained. But while there was a lot of discussion on the points on why the change was a bad idea, almost no one seems to have bothered detailing points why it's a good idea and comparing them to the negative point.
Reading through the discussion, the only person who bothered making a single point on why it was a good idea was Jack Phoenix:
I'm going to assume that the lowercase "skinname" in $IP/skins/skinname/SkinName.php is just a typo and you meant it to be CamelCased as "SkinName". If and when so, yes, this is what should be our recommended way of doing it. CamelCase is how we name things consisting
of
multiple words (i.e. BlueSky, DuskToDawn, HowTo, ...), so it's only reasonable to use CamelCase here too. Having written and cleaned up many skins myself, this is the naming convention I prefer and that seems
natural
right from the start.
For why it's a bad idea, I made various other points, some of the remaining ones summed up:
- Unlike extension names, skin names are used in a variety of locations which upper case letters are either considered bad or problematic, such as i18n keys, and keys to $wgValidSkinNames which is both used in those i18n keys and has to be lower cased if we want to make &useskin= case insensitive. So the lower cased keys will never actually go away.
- ((Not a point in favour, but when someone pointed out that extensions already have 3 names I pointed out I explained that in most cases they actually only have 1, at most 2))
- Skins only expose 2 names to site admins and users, the canonical lower case key and a fully translatable key. Because the lower case key doesn't actually disappear exposing a CamelCase name now gives skins 3 names to deal with - more than people installing extensions deal with - and makes 2 of those names "canonical" names that are supposed to uniquely identify the skin outside of the internals.
((Btw, I'll also point out that if we ever managed to implement real class-less skins using templates our current only need for CamelCase names - skin class names - would actually disappear))
As the discussion and attempts to implement this move on: Krinkle chimes in that skins should only have a lowercase non-ASCII id and a localized skin name. I help out reviewing the various patchsets trying to implement this, giving normal reviews, and also giving tips on how to properly write a skin that uses an upper case directory (because most people here trying it don't know how to do so without doing things like completely breaking the IE compatibility features in a skin). Bartosz agrees to leave skin directory names as-is while moving stuff out of core, at least while the discussion hasn't finished.
Then all of a sudden, while the discussion is still open, it's reported that someone who isn't even a maintainer for skin related stuff +2s something that changes skinname -> SkinName directories for some core skins, and documentation starts getting changed to say that skins use CamelCased directory names.
As unhappy as I am with my job revolving around themes and skins, I'm probably currently the one with the most knowledge on how the internals of our skinning system work. And it feels like me and one of the +2 maintainers for this skinning stuff in core are being ignored.
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://danielfriesen.name/]
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
I finally implemented most of the ideas discussed here into documentation, and I think I settled on something that should be acceptable to everyone. The canonical recommended way to do skins, and one we hopefully all agree on, is now described at https://www.mediawiki.org/wiki/Manual:Skinning#Skin_structure. Thanks for the comments!
I just wanted to thank Bartosz for all the good work he's done here! It's great that finally installing a skin is consistent with installing an extension :)
I wonder if we should create a skin boilerplate repository, to make it really easy for people to get started making their own skins?
On Sat, Jul 12, 2014 at 6:30 PM, Bartosz Dziewoński matma.rex@gmail.com wrote:
I finally implemented most of the ideas discussed here into documentation, and I think I settled on something that should be acceptable to everyone. The canonical recommended way to do skins, and one we hopefully all agree on, is now described at https://www.mediawiki.org/wiki/Manual:Skinning#Skin_structure. Thanks for the comments!
-- Matma Rex
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On 14/07/14 18:41, Jon Robson wrote:
I just wanted to thank Bartosz for all the good work he's done here! It's great that finally installing a skin is consistent with installing an extension :)
I wonder if we should create a skin boilerplate repository, to make it really easy for people to get started making their own skins?
Theoretically that's what the skins/Example repository should be for, but it seems to be empty at present.
-I
Thanks for the info Isarra!
Does anyone want to make it not empty...? :)
I'll personally +2 any changes if the author sends me a private email to the patchsets! On 15 Jul 2014 07:39, "Isarra Yos" zhorishna@gmail.com wrote:
On 14/07/14 18:41, Jon Robson wrote:
I just wanted to thank Bartosz for all the good work he's done here! It's great that finally installing a skin is consistent with installing an extension :)
I wonder if we should create a skin boilerplate repository, to make it really easy for people to get started making their own skins?
Theoretically that's what the skins/Example repository should be for, but it seems to be empty at present.
-I
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On Tue, 15 Jul 2014 16:38:52 +0200, Isarra Yos zhorishna@gmail.com wrote:
On 14/07/14 18:41, Jon Robson wrote:
I wonder if we should create a skin boilerplate repository, to make it really easy for people to get started making their own skins?
Theoretically that's what the skins/Example repository should be for, but it seems to be empty at present.
Yes, it's on my to-do list. I'm planning to do this tomorrow.
Late, but done: https://gerrit.wikimedia.org/r/#/c/147690/ . I'd love if some more people glanced at it before i make it "official".
-- Matma Rex
And here it is: https://www.mediawiki.org/wiki/Skin:Example and https://git.wikimedia.org/tree/mediawiki%2Fskins%2FExample.git
Thank you for the comments. It's nice knowing that somebody cares about the work I've been doing with this :)
\o/
On Mon, Jul 21, 2014 at 11:28 AM, Bartosz Dziewoński matma.rex@gmail.com wrote:
And here it is: https://www.mediawiki.org/wiki/Skin:Example and https://git.wikimedia.org/tree/mediawiki%2Fskins%2FExample.git
Thank you for the comments. It's nice knowing that somebody cares about the work I've been doing with this :)
-- Matma Rex
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
wikitech-l@lists.wikimedia.org