New revs - Public comments

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #450, on October 3rd, 2012, 11:44 PM »
Quote from Arantor on October 2nd, 2012, 11:38 PM
Not entirely. The error it fails on, at the first hurdle is the wesql is not loaded. But that's symptomatic of a larger problem - it isn't going to get database settings it can actually use until the beginning of the third step (step 1 = welcome + checking for fundamentals, step 2 = gathering database settings, step 3 is getting other stuff)
It has other issues... Like, defining WEDGE_INSTALLER in the first few steps when they're called at the same time -- it generates a warning so I had to add a defined() test before defining it.

Also... Plenty of other little bugs. Like, in rev 1710 I committed an unfortunate change to the ban groups table structure -- by renaming id_ban_group to id_ban_id_group. Probably a failed search & replace...!

Anyway -- I've successfully installed a copy without any bugs.

I should be able to make a commit tomorrow. I'm also going to commit a few changes to Welcome.template.php -- it's been left alone for so long, it's become very ugly...

BTW would it be possible to have you write a small plugin for wedge.org...? I'd just like to move the rev.txt and Facebook code to a plugin so that it doesn't end up by mistake in our final files... ;)
That's in index.template.php, of course.
Posted: October 3rd, 2012, 11:33 PM

Any ideas how to fix that one..?

8: Undefined index: latestRealName
File: /Sources/Subs.php
Line: 1534
(And more.)

It's the common_stats code. Very likely a bug caused by updateSettings being disabled during the install process..? But it should be enabled at that point, shouldn't it...?

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #451, on October 3rd, 2012, 11:56 PM »
Quote
It has other issues... Like, defining WEDGE_INSTALLER in the first few steps when they're called at the same time -- it generates a warning so I had to add a defined() test before defining it.
It wasn't meant to be a final solution. I only really cared enough about it actually letting me install.
Quote
Also... Plenty of other little bugs. Like, in rev 1710 I committed an unfortunate change to the ban groups table structure -- by renaming id_ban_group to id_ban_id_group. Probably a failed search & replace...!
I did notice it threw an error but I wasn't overly bothered, knowing that the plan is to remove those anyway in the end.
Quote
BTW would it be possible to have you write a small plugin for wedge.org...? I'd just like to move the rev.txt and Facebook code to a plugin so that it doesn't end up by mistake in our final files...
Sure, just trying to work out how best to do that. What occurs to me is me is that while it's possible to insert the SVN revision, just by munging $txt['copyright'] easily enough - inserting the Facebook bit is not quite so easy. What occurs to me as a consequence is to build an array in $context for the items that appear in the footer, which can be extended easily from a plugin.

There are only so many things that can be done in a plugin - either creating a template within some kind of template layer, or replacing the template in its entirety, or we have some structure that can be manipulated inside the template. It might be nice to have a $context variable that can be updated, though, in case people want to have custom copyrights in the footer (even if we think it's generally unnecessary when having the credits page)

I am also inclined to make the revision number available from the admin panel rather than being a file, just seems like it might be easier (and faster) - but I'm happy to make it either way.
Quote
Any ideas how to fix that one..?

8: Undefined index: latestRealName
File: /Sources/Subs.php
Line: 1534
(And more.)

It's the common_stats code. Very likely a bug caused by updateSettings being disabled during the install process..? But it should be enabled at that point, shouldn't it...?
It's completely that. The final step - DeleteInstall - is the step that calls updateStats which calls updateSettings, and is the point of not setting WEDGE_INSTALLER on every single install step.
When we unite against a common enemy that attacks our ethos, it nurtures group solidarity. Trolls are sensational, yes, but we keep everyone honest. | Game Memorial

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #452, on October 4th, 2012, 11:31 AM »
Quote from Arantor on October 3rd, 2012, 11:56 PM
It wasn't meant to be a final solution. I only really cared enough about it actually letting me install.
Well, I'm getting to it personally... As of now, except for the updateSettings problem for stats, everything is okay, and I've overhauled the install screen in a fashion that makes it now look cool. (Well, basically, I've mostly added a large size Wedge logo in the top left corner and added an easy gradient to the progress bar... :lol:)

Oh, there's also a bug with my clean_cache code... It doesn't seem to work, ah ah. At least with folders -- I added code to remove empty folders but it doesn't seem to work, even after force-deleting index.php, hmm...
Quote
I did notice it threw an error but I wasn't overly bothered, knowing that the plan is to remove those anyway in the end.
You should have reported it, still... ;) If only because it was my fault... I hate it when I add bugs.
Quote
Sure, just trying to work out how best to do that. What occurs to me is me is that while it's possible to insert the SVN revision, just by munging $txt['copyright'] easily enough - inserting the Facebook bit is not quite so easy. What occurs to me as a consequence is to build an array in $context for the items that appear in the footer, which can be extended easily from a plugin.
I'm not a big fan of making it easy to remove the copyright text, but I'd like to make it easy to ADD to it, precisely because it encourages people to add instead of removing stuff... Maybe a small template hook at that point?
Quote
There are only so many things that can be done in a plugin - either creating a template within some kind of template layer, or replacing the template in its entirety, or we have some structure that can be manipulated inside the template. It might be nice to have a $context variable that can be updated, though, in case people want to have custom copyrights in the footer (even if we think it's generally unnecessary when having the credits page)
Yes, the credits page would be the best place, but this is a discussion to be saved between plugin authors and plugin users, mostly -- do they feel their theme/plugin/poopoo warrants a footer copyright, or is it okay to have it in the credits page?
Quote
I am also inclined to make the revision number available from the admin panel rather than being a file, just seems like it might be easier (and faster) - but I'm happy to make it either way.
I don't know, I update the file manually when I do my uploads... To me, it's just a way to quickly tell people that I've updated the site. Having to go to the admin panel for that would add an unnecessary step in my upload process, I'd say...
Quote
It's completely that. The final step - DeleteInstall - is the step that calls updateStats which calls updateSettings, and is the point of not setting WEDGE_INSTALLER on every single install step.
Well then, why doesn't it work on my local install...?
Does it work on yours? Did you have the error last time you tried?

PantsManUK

  • [me=PantsManUK]would dearly love to dump SMF 1.X at this juncture...[/me]
  • Posts: 174
Re: New revs - Public comments
« Reply #453, on October 4th, 2012, 01:39 PM »
Quote from Nao on October 4th, 2012, 11:31 AM
[Yes, the credits page would be the best place, but this is a discussion to be saved between plugin authors and plugin users, mostly -- do they feel their theme/plugin/poopoo warrants a footer copyright, or is it okay to have it in the credits page?
As a plugins user, for "game changer" plugins (plugins that make massive changes to default Wedge, or effectively take over Wedge, like WedgeDesk) add to the footer, for your average plugin, the credits page. It's about "default view" (if you will); if your default view of the app is WedgeDesk (or WedgePortal (made up name, I assure you...), or whatever, f'rinstance), the footer needs to say WedgeDesk (or whatever), if your default view is Wedge, it needs to say Wedge. This makes the most sense to me...
« What is this thing you hoomans call "Facebook"? »

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #454, on October 4th, 2012, 03:15 PM »
Quote
I'm not a big fan of making it easy to remove the copyright text, but I'd like to make it easy to ADD to it, precisely because it encourages people to add instead of removing stuff... Maybe a small template hook at that point?
This is why I was thinking about creating a variable with it in, and then allow hooks to add to it (but specifically not able to modify the properties of that variable ;))

I don't really want to put it in the template because that would mean themes could prevent that being applied.
Quote
Yes, the credits page would be the best place, but this is a discussion to be saved between plugin authors and plugin users, mostly -- do they feel their theme/plugin/poopoo warrants a footer copyright, or is it okay to have it in the credits page?
This is one of those really huge debates that won't go away. Themes modify every page by definition, thus having the credit there is totally applicable. But plugins don't generally modify every page, in which case footer changes are bordering on inappropriate. It's almost entirely then about promotion and making money (like vbgamer and Nibogo are so fond of)
Quote
I don't know, I update the file manually when I do my uploads... To me, it's just a way to quickly tell people that I've updated the site. Having to go to the admin panel for that would add an unnecessary step in my upload process, I'd say...
This is why I thought I'd ask before just doing it ;)
Quote
Well then, why doesn't it work on my local install...?
Does it work on yours? Did you have the error last time you tried?
I had the error, figured out why, applied the test to check, and never re-tested it, assuming it would work >_<

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #455, on October 4th, 2012, 05:55 PM »
- I'm okay with adding a $context var next to $txt['copyright'] or whatever. :)
- I'm just not confident enough with plugin writing to even be able to do something as simple as hooking into that variable...

- Also, just wanted to say that locally, I'm finished with the install bugs. They're 100% fixed (hopefully), I even fixed a few bugs that weren't visible, and also fixed clean_cache() to work properly on sub-folders (I'm afraid it didn't cut it before then...)
I'm just left with committing the stuff... And it's quite a huge commit, intertwined with many CSS tweaks done over the week (or simply today, ah.) I'm actually tempted to just commit everything in a batch, rather than split files such as index.css which would need to be committed several times for thematic reasons... :-/

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #456, on October 4th, 2012, 05:59 PM »
Quote
- I'm okay with adding a $context var next to $txt['copyright'] or whatever.
Well, I was thinking that wetem could contain two extra methods: one for adding new values to a list and one for outputting those items - that way there's no ability to tamper with the contents because the contents are never pulled into a place they can ever be modified, yay for classes with proper variable scoping.

Doing it this way means it cannot be tampered with - putting it in $context means it can be tampered with.
Quote
- I'm just not confident enough with plugin writing to even be able to do something as simple as hooking into that variable...
I can understand that. What I think I'll do then is not only write the plugin but also go through and explain how all of it works, in the hopes that it helps.

Eh, just commit it and let the deities of computing sort it out :lol:

live627

  • Should five per cent appear too small / Be thankful I don't take it all / 'Cause I'm the taxman, yeah I'm the taxman
  • Posts: 1,670
Re: New revs - Public comments
« Reply #457, on October 5th, 2012, 12:53 AM »
CSS still ends up in 404 errors... will see what's up.

(JS seems fine)
Posted: October 5th, 2012, 12:31 AM

I only see the installer cache CSS folders.
Posted: October 5th, 2012, 12:34 AM

Had to update Settings.php. Problem fixed.
A confident man keeps quiet.whereas a frightened man keeps talking, hiding his fear.

Sara

  • Walking Contradiction
  • Posts: 41
Re: New revs - Public comments
« Reply #458, on October 5th, 2012, 02:41 AM »
Quote from Arantor on October 4th, 2012, 03:15 PM
This is one of those really huge debates that won't go away. Themes modify every page by definition, thus having the credit there is totally applicable. But plugins don't generally modify every page, in which case footer changes are bordering on inappropriate. It's almost entirely then about promotion and making money (like vbgamer and Nibogo are so fond of)
I agree, the credits page is sufficient.  The credits page can be long without tackiness.  Putting it in the footer of every page will add on extra bytes and might be extra cruft to a theme in general.  In fact, I prefer the credits page because it's well presented, and people are more apt to click and find out more about a plugin. </2 cents>

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #459, on October 5th, 2012, 08:57 AM »
@live> Generally speaking, you might want to be careful with your current local installs everytime you see a commit that modifies the other/ folder, especially the install.* and Settings files ;)

So, guys, what do you think about the new installer tweaks...?

Apart from the design, I'm also considering doing something smart: launching the Ajax test automatically instead of upon a click, and showing the results directly in the current window, i.e. a good opportunity to use $().load()... :P

@Pete & Sara> Hmm... So, let's sort it out: instead of trusting modders not to add links carelessly in the footer, we should prevent them from doing so at all. This goes through the use of private vars. We could possibly even test for the length of outputted strings, and if it's too long or its base64 version or whatever doesn't have a specific bit of string in it we can reset it, blah blah blah... (I'm trying to stick to trusting people, but I guess we can go either way eh..? :^^;:)

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #460, on October 5th, 2012, 03:25 PM »
I like the new tweaks, it looks nice :)

Yes, I agree that the installer test should be done like that - but let's go one better. If it doesn't pass for whatever reason, automatically disable the option.
Quote
So, let's sort it out: instead of trusting modders not to add links carelessly in the footer, we should prevent them from doing so at all. This goes through the use of private vars. We could possibly even test for the length of outputted strings, and if it's too long or its base64 version or whatever doesn't have a specific bit of string in it we can reset it, blah blah blah... (I'm trying to stick to trusting people, but I guess we can go either way eh..? :^^;:)
Well, doing this is easy enough; wetem can have control over it. The one thing that it would mean is that it would be... difficult to inject the SVN version number the way you currently have it in the footer, though in theory if you can get to $txt['copyright'] before it gets inserted into wetem (or wherever, but principally wetem), you can fudge it by inserting something on the end.

This is essentially the problem: whatever method we leave in for ourselves to use here will be exploitable elsewhere. But I think that's a compromise we can live with ;)

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #461, on October 6th, 2012, 03:10 PM »
Quote from Arantor on October 5th, 2012, 03:25 PM
I like the new tweaks, it looks nice :)
(And I'm probably going to keep that logo... It looks good at large sizes, and for small sizes I can always use whatever we have these days, as it's the same logo, only what's around it changes.)
Quote
Yes, I agree that the installer test should be done like that - but let's go one better. If it doesn't pass for whatever reason, automatically disable the option.
It seems like a given to me :P
But the problem is that as soon as I'm starting work on that, I'll get the idea to do the same for pretty URL support discovery (i.e. call for /whatever/ through Ajax and see if $.load() get something in return...), and I have a feeling I'll be spending a week on all of this... :^^;:
My personal goal is to have a 'releasable' alpha version by next week. (Which we'll release in private first anyway, but you get my point...)
Quote
Well, doing this is easy enough; wetem can have control over it. The one thing that it would mean is that it would be... difficult to inject the SVN version number the way you currently have it in the footer,
I don't mind changing the way it's shown... I just want to have it somewhere, preferably in the footer, and injected through a plugin.
Quote
This is essentially the problem: whatever method we leave in for ourselves to use here will be exploitable elsewhere. But I think that's a compromise we can live with ;)
Anyone's free to change $txt['copyright'] but we can always test for its contents in wetem...
Re: New revs - Public comments
« Reply #462, on October 8th, 2012, 05:00 PM »
Okay... Just had a couple of ideas I need to get opinions about.

- Loading settings: currently, we're getting strings, and for some we need to unserialize them, so we do that... I was thinking, if a plugin wants to save some settings, maybe we should recommend that they save them into a single variable because it's 'cleaner' that way, i.e. into an array... But because people might not want to do the serialize work, here's what we could do: we load $settings, and then iterate through it, searching for '{' in strings, if found, $arr = @unserialize($string), and if $arr !== false, then use $arr instead of the serialized string. Then, at updateSettings time, we can simply do an is_array() test, and if true, serialize the string, but still return the array instead of the serialized string of course.

What do you think...?
The only place where this would slow down performance is the strpos() on every single variable, or we could just directly try to unserialize them without a test, it warrants a test anyway... Other than that, it'd make life easier for everyone, I think.

- The sidebar... I've been postponing for a couple of weeks a skin change to Weaving that actually pushes the linktree inside the content area, because I figured that it's easier to get it outside of the content area with CSS (a negative margin, basically), than inside it. I've been trying to have Wuthering be the example of putting the linktree outside of the content area, but it doesn't work as well as I thought because the table-cell status of divs makes it impossible to adjust the sidebar without cancelling its table-cell status (which allows it to extend to the bottom of the page.)
So, basically, I have a few choices left...
- Give up on table-cell for the sidebar, like I suggested earlier, and use other tricks instead,
- Use a <skeleton> to move the linktree around in Wuthering, rather than a CSS change (this is probably what makes the most sense, but I wanted to show that it was possible to do with CSS...)
- Or just give up on putting the linktree above the sidebar & content, just have it sit above content and beside the sidebar as per usual.

What do you think...? (If anything.)

PS: I posted that to the wrong topic, but at least it reminds me that I have a couple of comments to make on the loadLanguage and capitalization changes...

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #463, on October 8th, 2012, 05:08 PM »
It doesn't make life any easier, on the contrary it makes plugin code more complicated because unless they're using a simple page (where we can hide the real complexity away from them), they have to do fudge things themselves. There are already examples in SMF and Wedge of doing crap like this (cf the warning settings, post moderation settings) only without being serialised, just comma-separated and that's messy enough.

All it does is keep things marginally smaller in the database, it doesn't make anything easier per se. I'd honestly rather leave it alone because the extra complexity, the extra performance hit for a marginal (at best) saving of space with no improvement in convenience for anyone, and the potential to introduce new bugs unnecessarily puts me off. It is an interesting idea but it's got way more downsides than upsides.


As far as the sidebar, I'm not fussed either way, just whatever is most flexible for theme designers.

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #464, on October 8th, 2012, 05:59 PM »
Quote from Arantor on October 8th, 2012, 05:08 PM
It doesn't make life any easier, on the contrary it makes plugin code more complicated because unless they're using a simple page (where we can hide the real complexity away from them), they have to do fudge things themselves. There are already examples in SMF and Wedge of doing crap like this (cf the warning settings, post moderation settings) only without being serialised, just comma-separated and that's messy enough.
I don't understand... Why can't we simply change these variables to use arrays instead, and have them go through the code I suggested?
I personally had a few times where I was going to store an array into $settings, and then thought to myself, "oh wait, I can't, I'm supposed to send a string instead..."
And if you're writing a plugin that accesses a serialized array in the settings, you don't want to have to unserialize it yourself, because when are you supposed to do that..? As a plugin author I'd expect Wedge to do it for me. (But then again that's just me...)
Imagine if you're accessing your $settings var in multiple places. Do you have to do an unserialize test on it everytime you use it..?
Quote
All it does is keep things marginally smaller in the database, it doesn't make anything easier per se. I'd honestly rather leave it alone because the extra complexity, the extra performance hit for a marginal (at best) saving of space with no improvement in convenience for anyone, and the potential to introduce new bugs unnecessarily puts me off. It is an interesting idea but it's got way more downsides than upsides.
I don't see how it could add any bugs. @unserialize will simply return false if it can't turn the string into an array. Meaning that it isn't an array at all.
Quote
As far as the sidebar, I'm not fussed either way, just whatever is most flexible for theme designers.
Hmm... That's exactly where I'm not too sure ;)
Posted: October 8th, 2012, 05:54 PM

Just did a quick test...

Code: [Select]
global $settings;
$t = microtime(true);
foreach ($settings as $truc)
if (strpos($truc, '{') !== false)
$truc = @unserialize($truc);
echo microtime(true)-$t;

Returns in around 1 millisecond on my local setup. About 2 if I remove the strpos() test, meaning it's best to avoid calling unserialize for nothing.

Is that worth the delay then..?

:edit: At this point in the code, they're already unserialized. I should test for another...