Right now in the skins system (if you consider vector part of the skins system) we have two parallel methods of adding tabs to the page: - Into content_actions via SkinTemplateTabs, SkinTemplateBuildContentActionUrlsAfterSpecialPage, and SkinTemplateContentActions, - Into vector's navigation_urls via SkinTemplateNavigation (the missing two hooks should be added)
The only important difference between these (besides some vector specific stuff that can stay in vector) is that content_actions is a flat array, and navigation_urls is an array of arrays organized into categories. Besides that, they are basically mirrors of each other with the same functional purpose, but you need to add tabs to both of them to avoid something not showing up in vector. There's also the misfortune that other skins can't take advantage of the organized navigation_urls because the actual implementation (which is basically a reimplementation of buildContentActionUrls with code duplication) without being a vector subskin because the code in question is inside of vector.
Right now we have extensions using both methods of adding tabs to the page, code duplication on their part. And a few extensions that are broken in vector because they haven't added the hooks. Doing a quick grep, it appears the following extensions are missing vector support: Oversight, CommentPages, Todo, WikiTrust, Tasks, CategoryTree, DeleteQueue, Wikidata, Imagetabs, purgetab, Tab0, AuthorProtect, TidyTab, Purge, SpecialTalk
Shouldn't be to hard to fix, especially if we fix the bug of missing hooks for navigation_urls.
Now onto my focal point. As I've been improving the skin system trying to pull out the thorns that make building skins troublesome and mesh in new features and helpers which are missing, I'd like to remove the content_actions hooks and deprecate content_actions in 1.18 and start using navigation_urls style data everywhere. Since content_actions and navigation_urls are the same, content_actions can be built by having SkinTemplate take the navigation_urls data and flatten it into a single array. Similarly to how I already have $wgFooterIcons work and fold it for some skins like Monobook which don't organize it the way vector does.
The effects will be like this: - The three content_actions related hooks will no longer work in 1.18, thus extensions that haven't started supporting vector tabs will also stop showing tabs in other skins - In their place extensions will use 3 navigation_urls related hooks (most extensions are already using the one hook available) - Extension code for those already using both forms of hooks will stay the same, the only difference being that 1.18 will use the navigation_urls related hooks and the content_actions related ones will become redundant code which the extensions can keep for back compat but drop once they stop supporting pre-1.18 installations - All standard skins will be using navigation_url based data and content_actions will be available but deprecated - 3rd party skins will still function using content_actions but it would be preferred for them to be updated to use the new BaseTemplate and use the helpers in there (a navigation_urls related one would be added) once they don't need to support pre-1.18 - SkinTemplatePreventOtherActiveTabs will probably still work, though I may want to find a cleaner method to transition to (ie: one that says "this is the active tab" rather than "don't make other tabs active").
Any comments, rejections?
On Mon, Dec 27, 2010 at 9:02 AM, Daniel Friesen lists@nadir-seen-fire.com wrote:
The effects will be like this:
- The three content_actions related hooks will no longer work in 1.18,
thus extensions that haven't started supporting vector tabs will also stop showing tabs in other skins
- In their place extensions will use 3 navigation_urls related hooks
(most extensions are already using the one hook available)
If it wouldn't be too much extra work, it would be preferable to keep back-compat for the content_actions hooks -- have the code rewrite any changes to content_actions to comparable changes to navigation_urls. But only if it's not too messy (I haven't looked at the code). Other than that, I'm entirely in favor of harmonizing the skin implementations.
On 10-12-27 09:40 AM, Aryeh Gregor wrote:
On Mon, Dec 27, 2010 at 9:02 AM, Daniel Friesen lists@nadir-seen-fire.com wrote:
The effects will be like this:
- The three content_actions related hooks will no longer work in 1.18,
thus extensions that haven't started supporting vector tabs will also stop showing tabs in other skins
- In their place extensions will use 3 navigation_urls related hooks
(most extensions are already using the one hook available)
If it wouldn't be too much extra work, it would be preferable to keep back-compat for the content_actions hooks -- have the code rewrite any changes to content_actions to comparable changes to navigation_urls. But only if it's not too messy (I haven't looked at the code). Other than that, I'm entirely in favor of harmonizing the skin implementations.
While writing that e-mail I did contemplate an idea on how the old content_actions hooks could potentially just be given a single category and say drop tabs into an "other" category. But then I made a few realizations. Firstly, an "other" category will not work nicely with vector's grouping of tabs... Secondly, restricting to any specific category will not be compatible with hooks trying to remove from another category. But worst of all... because extensions want to maintain backwards compatibility for awhile, the extensions that are right now trying to be backwards and forwards compatible (the way we want them to be), or rather actually work in both monobook and vector class skins will start inserting duplicate tabs into the page if we do something like that.
It really shouldn't be too hard to take any implementation using content_actions hooks and make them work with navigation_urls hooks (once we add the two missing hooks) since they're really the same hooks, with the same data, the only difference is with the navigation_urls hooks you use something like $nav_urls["actions"][...] instead of $content_actions[...] for your code... so it's really just copying the code, deciding which category the tab belongs in, and making minor tweaks.
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://daniel.friesen.name]
Well OK, just hope none of what I wrote breaks.
function JidanniLessRedContentActions($sktemplate,$content_actions){ //Besides Monobook, (our target), this even also gets run by Vector skin but doesn't affect it apparently 11/2010 if(array_key_exists('talk', $content_actions)&&'new'==$content_actions['talk']['class']&& !$sktemplate->mTitle->quickUserCan('createtalk')){unset($content_actions['talk'],$content_actions['watch']);} if(array_key_exists('nstab-category',$content_actions)&&'selected new'==$content_actions['nstab-category']['class']){ $content_actions['nstab-category']['class']='selected';} return true;} $wgHooks['SkinTemplateTabs'][]='JidanniLessRedContentActions'; //Bug 17963 function JidanniLessRedContentActionsVectorTypeSkins($sktemplate,$links){ if(isset($links['namespaces'])&& is_array($links['namespaces'])&& !$sktemplate->mTitle->quickUserCan('createtalk')){ foreach(array_keys($links['namespaces']) as $ns){ if(strpos($ns,'talk')!==false){ if(isset($links['namespaces'][$ns]['class'])&& 'new'==$links['namespaces'][$ns]['class']){ unset($links['namespaces'][$ns]);}}} if(isset($links['actions']['watch'])){unset($links['actions']['watch']);}} if(isset($links['namespaces']['category']['class'])&& 'selected new'==$links['namespaces']['category']['class']){ $links['namespaces']['category']['class']='selected';} return true;} $wgHooks['SkinTemplateNavigation'][]='JidanniLessRedContentActionsVectorTypeSkins';
Maybe I can even remove the first function soon.
On 11-01-01 09:10 PM, jidanni@jidanni.org wrote:
Well OK, just hope none of what I wrote breaks.
function JidanniLessRedContentActions($sktemplate,$content_actions){ //Besides Monobook, (our target), this even also gets run by Vector skin but doesn't affect it apparently 11/2010 if(array_key_exists('talk', $content_actions)&&'new'==$content_actions['talk']['class']&& !$sktemplate->mTitle->quickUserCan('createtalk')){unset($content_actions['talk'],$content_actions['watch']);} if(array_key_exists('nstab-category',$content_actions)&&'selected new'==$content_actions['nstab-category']['class']){ $content_actions['nstab-category']['class']='selected';} return true;} $wgHooks['SkinTemplateTabs'][]='JidanniLessRedContentActions'; //Bug 17963 function JidanniLessRedContentActionsVectorTypeSkins($sktemplate,$links){ if(isset($links['namespaces'])&& is_array($links['namespaces'])&& !$sktemplate->mTitle->quickUserCan('createtalk')){ foreach(array_keys($links['namespaces']) as $ns){ if(strpos($ns,'talk')!==false){ if(isset($links['namespaces'][$ns]['class'])&& 'new'==$links['namespaces'][$ns]['class']){ unset($links['namespaces'][$ns]);}}} if(isset($links['actions']['watch'])){unset($links['actions']['watch']);}} if(isset($links['namespaces']['category']['class'])&& 'selected new'==$links['namespaces']['category']['class']){ $links['namespaces']['category']['class']='selected';} return true;} $wgHooks['SkinTemplateNavigation'][]='JidanniLessRedContentActionsVectorTypeSkins';
Maybe I can even remove the first function soon.
That code is a mess... but as long as you have that second hook things will work. Monobook and other skins will use the first hook and Vector will use the second up to 1.17, in 1.18 all skins will use the second hook. I would suggest not depending on the exact order and equality of class strings, it's possible that future code changes may add new classes or change the order of those. My code changes might affect your 'watch' use in that code, don't know if that's a good thing or not. Previously Vector built the watch tab conditionally based on the use watch icon setting. If it was enabled watch ended up inside views instead of actions. Now the watch tab is 'always' put into actions... vector relocates it to views and turns the icon on afterwards if that feature is enabled.
And yes, in 1.17 and before SkinTemplateTabs is run in vector but ignored... that's because it's SkinTemplate that runs it, so all of our normal skins run it whether they use it to build tabs or not. It's not really anything special if you look over SkinTemplate... SkinTemplate basically preloads whatever QuickTemplate based template you have with a bunch of skin keys with data and then executes the template... it doesn't build them on demand or anything.
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://daniel.friesen.name]
"DF" == Daniel Friesen lists@nadir-seen-fire.com writes:
DF> That code is a mess... I would suggest not depending on the exact order and DF> equality of class strings, it's possible that future code changes DF> may add new classes or change the order of those.
OK, here is what I reduced it to. Note I totally depend on the var_dumps to see what is coming my way, so perhaps you could tighten/loosen up my code to make it more future-proof. Thanks.
function JidanniLessRedNavigation($sktemplate,$links){ // var_dump('BEFORE',$links); if(!$sktemplate->mTitle->quickUserCan('createtalk')){ foreach(array_keys($links['namespaces']) as $ns){ if(strpos($ns,'talk')!==false){ if('new'==$links['namespaces'][$ns]['class']){ unset($links['namespaces'][$ns]);}}}} if(isset($links['namespaces']['category']['class'])&& 'selected new'==$links['namespaces']['category']['class']){ $links['namespaces']['category']['class']='selected';} // var_dump('AFTER',$links); return true;} $wgHooks['SkinTemplateNavigation'][]='JidanniLessRedNavigation';
On 11-01-11 05:45 PM, jidanni@jidanni.org wrote:
"DF" == Daniel Friesenlists@nadir-seen-fire.com writes:
DF> That code is a mess... I would suggest not depending on the exact order and DF> equality of class strings, it's possible that future code changes DF> may add new classes or change the order of those.
OK, here is what I reduced it to. Note I totally depend on the var_dumps to see what is coming my way, so perhaps you could tighten/loosen up my code to make it more future-proof. Thanks.
function JidanniLessRedNavigation($sktemplate,$links){ // var_dump('BEFORE',$links); if(!$sktemplate->mTitle->quickUserCan('createtalk')){ foreach(array_keys($links['namespaces']) as $ns){ if(strpos($ns,'talk')!==false){ if('new'==$links['namespaces'][$ns]['class']){ unset($links['namespaces'][$ns]);}}}} if(isset($links['namespaces']['category']['class'])&& 'selected new'==$links['namespaces']['category']['class']){ $links['namespaces']['category']['class']='selected';} // var_dump('AFTER',$links); return true;} $wgHooks['SkinTemplateNavigation'][]='JidanniLessRedNavigation';
Using in_array and array_diff on an exploded array should work fine. I suppose we could use some sort of Html::hasClass helper method.
Personally I wish we never even started using 'class' like this. Proper 'selected' => true, 'new' => true keys that get mapped to classes just before going off to the skin, or even better, let the skin actually choose which class it's going to use for selected things.
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://daniel.friesen.name]
"DF" == Daniel Friesen lists@nadir-seen-fire.com writes:
DF> Using in_array and array_diff on an exploded array should work fine. OK, but that would take several times as many lines than my current:
foreach(array_keys($links['namespaces']) as $ns){ if(strpos($ns,'talk')!==false){ if('new'==$links['namespaces'][$ns]['class']){ unset($links['namespaces'][$ns]);}}}
and force me to hardwire in more specific knowledge of your current structure, no?
On 11-01-13 05:35 PM, jidanni@jidanni.org wrote:
"DF" == Daniel Friesenlists@nadir-seen-fire.com writes:
DF> Using in_array and array_diff on an exploded array should work fine. OK, but that would take several times as many lines than my current:
foreach(array_keys($links['namespaces']) as $ns){ if(strpos($ns,'talk')!==false){ if('new'==$links['namespaces'][$ns]['class']){ unset($links['namespaces'][$ns]);}}}
and force me to hardwire in more specific knowledge of your current structure, no?
I'm talking about on class strings. `in_array('new', explode(' ', $links['namespaces'][$ns]['class']));` rather than `'new'==$links['namespaces'][$ns]['class']` which will break should another class be added to that item. And you can use array_diff in a similar way to check for multiple classes
Still, I wish we never started using classes like this in the first place. These would have been much better off as array keys.
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://daniel.friesen.name]
Op 14 jan 2011, om 02:53 heeft Daniel Friesen het volgende geschreven:
On 11-01-13 05:35 PM, jidanni@jidanni.org wrote:
> "DF" == Daniel Friesenlists@nadir-seen-fire.com writes:
DF> Using in_array and array_diff on an exploded array should work fine. OK, but that would take several times as many lines than my current:
foreach(array_keys($links['namespaces']) as $ns){ if(strpos($ns,'talk')!==false){ if('new'==$links['namespaces'][$ns]['class']){ unset($links['namespaces'][$ns]);}}}
and force me to hardwire in more specific knowledge of your current structure, no?
I'm talking about on class strings. `in_array('new', explode(' ', $links['namespaces'][$ns]['class']));` rather than `'new'==$links['namespaces'][$ns]['class']` which will break should another class be added to that item. And you can use array_diff in a similar way to check for multiple classes
Still, I wish we never started using classes like this in the first place. These would have been much better off as array keys.
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http:// daniel.friesen.name]
I'm not sure about the context but I think using foreach as key => value makes more sense in instead of getting array_keys and getting the value by accessing the array (again) with the key.
See the following:
foreach( $links['namespaces'] as $ns => $value ) { if( strpos( $ns, 'talk' ) !== false ){ if( in_array( 'new', explode(' ', $value['class'] ) ) ) { unset( $links['namespaces'][$ns] ); } } }
Just fyi: The number of lines doesn't always represent performance gain.
And, from what I can see this loop would mark a namespace called "Stalk" incorrectly as "talk namespace".
For example * SiteName: StalkBot Wiki * Project namespace: StalkBot * Project talk namespace: StalkBot_talk or any word in any language containing the letters 'talk'.
OK thanks fellas, I now use function JidanniLessRedNavigation($sktemplate,$links){ foreach($links['namespaces'] as $ns=>&$value){ if($value['context']=='talk' && $value['class']=='new' && !$sktemplate->mTitle->quickUserCan('createtalk')){ unset($links['namespaces'][$ns]);} if($ns=='category' && $value['class']=='selected new'){ $value['class']='selected'; if(isset($links['actions']['watch'])){unset($links['actions']['watch']);}}} return true;} $wgHooks['SkinTemplateNavigation'][]='JidanniLessRedNavigation';
My specially clamped down radio scanning wiki was a 2007 WikiMania poster child http://jidanni.org/comp/wiki/article-category.html You can give it a whirl.
I could use in_array, explode, etc. but well, what I would be looking for is something nobody knows about yet, including me.
Maybe I should use isset()s every step of the way, lest users see embarrassing warnings one day when you fellows change something. Or perhaps no isset()s, that way users will notify me that something has changed in your structure, and I can adapt to get my function back working again.
jidanni@jidanni.org wrote in message news:87vd1p3891.fsf@jidanni.org...
Maybe I should use isset()s every step of the way, lest users see embarrassing warnings one day when you fellows change something. Or perhaps no isset()s, that way users will notify me that something has changed in your structure, and I can adapt to get my function back working again.
Isn't that what release notes are for?
--HM
On Sun, Jan 16, 2011 at 7:16 PM, Happy-melon happy-melon@live.com wrote:
jidanni@jidanni.org wrote in message news:87vd1p3891.fsf@jidanni.org...
Maybe I should use isset()s every step of the way, lest users see embarrassing warnings one day when you fellows change something. Or perhaps no isset()s, that way users will notify me that something has changed in your structure, and I can adapt to get my function back working again.
Isn't that what release notes are for?
Yes. And you probably shouldn't enable error reporting on production/public wikis anyway.
-Chad
C> On Sun, Jan 16, 2011 at 7:16 PM, Happy-melon happy-melon@live.com wrote:
Isn't that what release notes are for?
Say, how do you pros see what changed? Here is my extra stupid way. I do it every few weeks.
cp RELEASE-NOTES /tmp svn update diff --ignore-space-change -U0 /tmp RELEASE-NOTES
Often old lines are shown again, because somebody tidied their formatting. A svn diff wouldn't be any better.
The worst thing is each 1.17 to 1.18, 1.18 to 1.19 change, when the whole file changes.
So how do you folks track RELEASE-NOTES level changes (not source code level changes, too many), coinciding with your SVN updates?
On Wed, Jan 19, 2011 at 8:55 PM, jidanni@jidanni.org wrote:
So how do you folks track RELEASE-NOTES level changes (not source code level changes, too many), coinciding with your SVN updates?
I don't follow release notes, I'm subscribed to mediawiki-cvs.
It's way more informative.
-Chad
"C" == Chad innocentkiller@gmail.com writes:
C> I don't follow release notes, I'm subscribed to mediawiki-cvs. C> It's way more informative.
Ah, http://news.gmane.org/group/gmane.org.wikimedia.mediawiki.cvs 99 times overload for me though.
jidanni@jidanni.org wrote in message news:87mxmwfie4.fsf_-_@jidanni.org...
C> On Sun, Jan 16, 2011 at 7:16 PM, Happy-melon happy-melon@live.com wrote:
Isn't that what release notes are for?
Say, how do you pros see what changed? Here is my extra stupid way. I do it every few weeks.
cp RELEASE-NOTES /tmp svn update diff --ignore-space-change -U0 /tmp RELEASE-NOTES
Often old lines are shown again, because somebody tidied their formatting. A svn diff wouldn't be any better.
The worst thing is each 1.17 to 1.18, 1.18 to 1.19 change, when the whole file changes.
So how do you folks track RELEASE-NOTES level changes (not source code level changes, too many), coinciding with your SVN updates?
One generally gets information out of a release notes file by reading it. The purpose of release notes is to be a list of changes between versions; if I want to upgrade from 1.15 to 1.17, I read the 1.16 and 1.17 release notes, and I know all the changes I should be expecting. Why would I want the 1.16 release notes to contain changes which do not affect 1.16?
If you are determined to deploy bleeding-edge code on production sites, you will need to be a little more adventurous in getting hold of the latest changes. You can go to [1] and select the latest revision, and the last revision you checked out, and read the diff in a slightly prettier format. Pretty much by definition, there isn't a nicer way of reading them.
--HM
[1] http://svn.wikimedia.org/viewvc/mediawiki/trunk/phase3/RELEASE-NOTES
OK, I'm pretty happy with $ svn diff -r HEAD RELEASE-NOTES However that gives a backwards view, --- RELEASE-NOTES (revision 81238) +++ RELEASE-NOTES (working copy) I want --- RELEASE-NOTES (working copy) +++ RELEASE-NOTES (revision 81238) But $ svn help diff is written to abstrusely for me to figure out how. Do I have to hardwire version numbers into my command line? Thanks.
jidanni@jidanni.org wrote:
OK, I'm pretty happy with $ svn diff -r HEAD RELEASE-NOTES However that gives a backwards view, --- RELEASE-NOTES (revision 81238) +++ RELEASE-NOTES (working copy) I want --- RELEASE-NOTES (working copy) +++ RELEASE-NOTES (revision 81238) But $ svn help diff is written to abstrusely for me to figure out how. Do I have to hardwire version numbers into my command line? Thanks.
svn diff -r BASE:HEAD RELEASE-NOTES
wikitech-l@lists.wikimedia.org