New revs - Public comments

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #75, on August 30th, 2011, 01:09 AM »
Quote
Even less sure about the removal of the help icon saying why the password has to be entered again. Are you... sure it's related to the above...? I wouldn't say it is.
Bah, I was a bit too enthusiastic with the delete button. The extra string has the setting name in it, which is where I enthusiastically removed it.
Quote
I'm not 100% sure about the removal of admin confirmation though... Not that I don't get your point -- but I can see people complaining the option is no longer there, and having to tell them to use phpMyAdmin to change it...
I really don't see it as a problem: it is not the sort of setting that actively needs to be present, and I'd argue the number of people that actively need to use it is pretty small, small enough that it can be removed to prevent the idiocy of the rest.

See also where it was discussed - http://wedge.org/pub/feats/6846/more-stuff-for-the-removal-of/
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

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 #76, on September 3rd, 2011, 11:58 PM »
Code: [Select]
foreach (explode(',', strtolower($modSettings['disabledBBC'])) as $tag)

Wouldn't that do the explode for every disabled BB code?[1]?
 1. I think that's how Javascript does it.
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 #77, on September 4th, 2011, 12:22 AM »
I thought that foreach evaluated it only the first time to build the array. I smell a benchmark in the wings.

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 #78, on September 4th, 2011, 12:51 AM »
I did some research on this and it turns out that, indeed, PHP explodes it once and keeps it in memory.

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #79, on September 4th, 2011, 01:02 AM »
I wouldn't have made that change otherwise ;) SMF was using a temp var for nothing.

It must not be confused with for() where the second argument is always reevaluated.

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 #80, on September 4th, 2011, 02:26 AM »
Code: [Select]
// Parse the smileys within the parts where it can be done safely.
if ($smileys === true)
{
$message_parts = explode("\n", $message);
for ($i = 0, $n = count($message_parts); $i < $n; $i += 2)
parsesmileys($message_parts[$i]);

$message = implode('', $message_parts);
}

This is at the end of parse_bbc(). Is there a specific reason for parsing smileys on every line? Why not parse the whole message, which makes more sense?

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #81, on September 4th, 2011, 03:14 AM »
I assume it's to do with the code tags and similar where smileys shouldn't be parsed. Notice that it expressly doesn't do every line but every other line in the file.

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #82, on September 4th, 2011, 08:45 AM »
I myself have learned not to meddle too much with parse_bbc() doings... Usually they're there for a reason, too bad they're not documented ;)
I tried to comment the actions I'm performing in the new version of Aeva-Embed.php -- it's probably a good thing since I had a hard time diving back into the original code, even though I spent so much time on it :P

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 #83, on September 4th, 2011, 09:00 AM »
Quote from Nao on September 4th, 2011, 08:45 AM
I tried to comment the actions I'm performing in the new version of Aeva-Embed.php -- it's probably a good thing since I had a hard time diving back into the original code, even though I spent so much time on it :P
Six-month[1] code can look like  someone else wrote it.
 1. It's just a random number, the first one I thought of.

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #84, on September 4th, 2011, 09:26 AM »
It also reminds me that I don't need to prove the world anything... My code was already perfectly, excruciatingly godly years ago, it's not like I'm improving and I need to show it... :whistle:

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #85, on September 5th, 2011, 09:12 PM »
Quote
PS: Pete... I know you're not as used to committing as I am (), but could you please think of posting your changelogs...?
Um, if you're referring to r980, that was yours... My last commit was r972, which I posted... All the changes I'm making are local until I'm done with it...

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #86, on September 5th, 2011, 09:19 PM »
Oh. Sorry then. I'm not on my pc so I can't check whatever I screwed up. Plus I have another big commit coming up... Oh what a week.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #87, on September 5th, 2011, 09:21 PM »
No worries :) It's a big week for me too with the new plugin manager code...

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #88, on September 5th, 2011, 09:32 PM »
Keep up the good work :)

I saw where I made a mistake-- I'd simply looked at the previous post with rev 976, and posted the new one as 977... When I'd actually posted a 977 immediately after 976 and merged the two posts for good measure.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #89, on September 9th, 2011, 10:18 AM »
I haven't touched your latest stuff yet, still on the add-on infrastructure trail.

However, there is one thing I noted about the change to quick reply... I think I'd rather make it shown by default, rather than bigger but still hidden by default. It's a surprisingly common question on SMF, not helped by the fact that the process of changing it is relatively illogical at present, but I'm inclined to think it should be shown, on by default.