New revs - Public comments

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #495, on January 5th, 2013, 05:00 PM »
Quote from Arantor on January 5th, 2013, 04:10 AM
It might not be if all I wanted to do were update the timestamp. But touch can't guarantee the file is also empty, and I'd sort of like cache.lock to remain empty.
I mean, once it's created (through the first fopen/fclose call), why not just touch it..? Who's going to edit it and add stuff to it anyway..?
Quote
The key thing was to nail reserveName, IIRC it was called, which is gone along with reserveWord. I didn't see a need to go back and rename every little function that happens to share a word with the old feature name.
I double-checked, and it seems to be all fine in the end. :)
Quote
Quote
- Still against renaming .wmenu() to .dome(), right? :^^;: Well, it doesn't cost anything to ask...
You do what you have to do. As long as it doesn't break anything I'm working on, that's cool.
After some testing (sigh), .wmenu() and .dome() pretty much use the same amount of bytes in gzipped mode... It depends on other factors, but it's not worth doing it I guess.
Quote
Aside from the fact it's bytes that don't need to be there, there are times it will actually damage the content of the file being sent, potentially with attachments and CAPTCHAs and most of the time that one or other of these fail, that's the reason.
So, how about removing the ?> altogether..?
Quote
That's pretty much taken just from SMF. There was doubtless a reason it was done that way but I was never privy to it. If you're going to change it, be sure that it works exactly as before.
SMF had plenty of lame regexes that I fixed over the years, so this particular one was probably built by someone who didn't know about (?:) (non-capturing parenthesis)...
Quote
Again, such refactoring is cool, but only if we're absolutely sure it doesn't break anything. I'm still not entirely happy about the dropping of ENT_QUOTES on subject titles 'because it made it shorter' with the caveat that we have to go and put it back again in some places as opposed to just leaving it alone.
I don't remember that :P
But isn't entity fixing a feature of westr? So why not just rely on westr to call our thingies...? It's easy enough to set a method as a callback for preg_replace, after all.
Quote
That's just names. Makes no difference to me what it's called. It's logical, it works.
Then thus shall they be named... :P
Quote
When you have a proper nested array there, it looks natural. But when you have an array like that, doing it without looks more natural to me. Whatever works, I guess.
It's not about what looks natural here, it's mostly about following the SMF guidelines.
There was a hickup in the SMF 2.0 query conversion in that they committed a 'bad' copy with lots of bad automatic replacements, and had to modify them manually, one by one, for weeks... I remember that. I also remember that's pretty much when they started taking a lot of space for any single query (like, 3 lines of PHP for a single line query...)

So, anyway, I've gotten used to SMF's way, but have no qualms with yours either. What do we call? "Anything goes for the query call indenting format"...?
Quote
Firstly, heredocs or nowdocs?
I said heredocs :P
Quote
heredocs support 5.2+ while nowdocs are 5.3+ only. The key difference is in variable use - heredocs support parsing of variables like double quoted strings do, and just for fun, they're also at least as slow as double quoted strings, if not even slower. And it breaks indentation because the ending of the heredoc must be the first thing on the line, without any exception.
Okay, then dropping the idea! I only got it after re-reading through the commits... Some areas were a bit unreadable due to the string effect.

Dragooon

  • I can code! Really!
  • polygon.com has to be one of the best sites I've seen recently.
  • Posts: 1,841
The way it's meant to be

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #497, on January 5th, 2013, 06:44 PM »
Quote
I mean, once it's created (through the first fopen/fclose call), why not just touch it..? Who's going to edit it and add stuff to it anyway..?
The main reason is that it's a file in the cache folder that is not currently being excluded from the htaccess file. If I were to add it to the htaccess file as an exclusion (I need to add *.zip anyway), I would have no problem reverting it to a touch, but interestingly touch is actually 'buggy' under Windows - it won't change the modification time until PHP 5.3.0, while the fopen/fclose will force the file to be updated.

A certain amount of this is security by design; if I'm creating a file, I feel responsible for the content of that file, and it's my duty to make sure that file doesn't have anything in it that it shouldn't have.
Quote
So, how about removing the ?> altogether..?
I have no objections to that.
Quote
But isn't entity fixing a feature of westr? So why not just rely on westr to call our thingies...? It's easy enough to set a method as a callback for preg_replace, after all.
It is, but I don't think it does exactly the same fixing.
Quote
It's not about what looks natural here, it's mostly about following the SMF guidelines.
We can always make our guidelines. We HAZ TEH POWAR!
Quote
o, anyway, I've gotten used to SMF's way, but have no qualms with yours either. What do we call? "Anything goes for the query call indenting format"...?
I tend to go with whatever looks right. But I'm really not that fussed.
Quote
I said heredocs
I know you did but I wanted to be absolutely clear as a lot of people seem to conflate the two a lot.
Quote from Dragooon on January 5th, 2013, 06:34 PM
$("#menu").dome();

Oh..hell no.
Why not?
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

Dragooon

  • I can code! Really!
  • polygon.com has to be one of the best sites I've seen recently.
  • Posts: 1,841

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
A confident man keeps quiet.whereas a frightened man keeps talking, hiding his fear.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #500, on January 6th, 2013, 12:31 AM »
men u do me?

Sounds like something a lady of the night might wonder.

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

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #502, on January 6th, 2013, 12:54 AM »
If it hadn't been me that said it, it would have been someone else.

Oracle

  • Posts: 78

godboko71

  • Fence accomplished!
  • Hello
  • Posts: 361
Thank you,
Boko

Dragooon

  • I can code! Really!
  • polygon.com has to be one of the best sites I've seen recently.
  • Posts: 1,841

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #506, on January 7th, 2013, 08:03 PM »
Quote from Nao on February 15th, 2012, 05:25 PM
- Used bindEvents at one point, instead of reproducing the function. I just modified bindEvents to allow passing specific DOM elements to it.
So, this is an old change of mine... I liked the idea, still like it, but I just noticed that it's only being used twice in the entire project: once in the index template, for obvious reasons, and once in an admin page.
I tried removing it from both, replacing the admin one with a copy of the original bindEvents (it's not THAT long, and it's on only one page... admin one, on top of that), and instead of calling it at the end of the index template, I'm now doing a DOMContentLoaded call for it, which works just the same because basically these events are executed once the HTML is loaded, and after the script.js file is loaded anyway (so we already have everything under the hand at that point).
As a result, I'm saving about 10 gzipped bytes in script.js by moving the call to my DOMContentLoaded function[1], as well as about 6-7 gzipped bytes in all HTML pages.
I also moved $('select').sb() to that function, for the same reasons. This saves another few bytes in both sbox.js (part of script.js) and all HTML pages (because the call was also hardcoded in there, except when creating a new session under a browser other than IE 6-8, IIRC.)

So, all in all, I saved over 20 gzipped bytes from script.js and about 10-20 (hard to count) from all HTML pages.
Pretty happy with that, but there are two remaining issues:
- is it okay, or at least acceptable, to place that DCL function within an unrelated function...?
- can anyone think of any situation where powering up the menus, or the select boxes, or building the event list would better be placed where they used to be, rather than in a DCL function...?

Really dunno about all of this...
 1. Which, I'm afraid, and it's the only thing that bugs me a bug, is executed from within the menu building function... It's totally out of place, but this allows me to avoid building a $.fn.wmenu plugin to begin with. I don't know if I'll keep it that way...

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #507, on January 7th, 2013, 08:11 PM »
I'm not really fussed where it is, provided that it works and doesn't break anything.

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #508, on January 7th, 2013, 08:30 PM »
Okay good ;)
As for whether it works or not -- I guess we'll have to wait until we get some user feedback on these... (?)
I'm at least hoping it'll help fix an issue in the Android stock browser I got with select boxes. Dunno.

Oh, and I forgot to ask whether it was okay to get rid of bindEvents' parameter, since it was only used in one function and I don't see many situations where we would need to create new HTML elements on the fly with events attached to them, and without actually directly using .bind() or similar functions to attach these events. (Which is faster, considering that bindEvents() eventually calls .bind() for all events it can find...)

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #509, on January 7th, 2013, 08:32 PM »
I can't imagine many cases, no. I think it's rare enough that it's probably OK not to worry too much about it.