New revs - Public comments

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #510, on January 7th, 2013, 08:34 PM »
Yup.

Oh, and it's a bit scary to have Wedge define eves = {} at some point in the HTML code and have script.js rely on this without testing, but that's the beauty of DCL... :lol:

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #511, on January 7th, 2013, 08:37 PM »
Scary for you - as long as I can add events inline and add deferred JS with add_js(), I'm happy.
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

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #513, on January 7th, 2013, 11:30 PM »
Then it's fine :)

I'm looking at it in terms of the modder approach; I want to be able to just build stuff and rely on it working, I don't want to have to debug a ton of stuff to write a simple plugin - it sort of defeats the purpose ;)

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #514, on January 9th, 2013, 10:02 AM »
Well, I'll always be here to fix bugs in case they arise... I'm pretty much responsible for the frontend and all of the visual tricks.
That's what I did yesterday... Fixed a bug introduced by IE+Safari misbehaving with jQuery and conflicting with my ask() function. (Oh, and in case you don't follow the jQuery bug report I linked to, which is likely, I posted a request to fix it... Which they won't do. Nice. I've posted more arguments but I'm sure they'll be ignored as well... Maybe it's time to fork jQuery :lol:)
Re: New revs - Public comments
« Reply #515, on January 19th, 2013, 02:31 PM »
So, I'm looking at Pete's latest commits and I found something funny... Another of those 'coincidences' I noticed earlier ;)

Last night I was fixing a bug (which I have yet to commit), related to Wedge cutting off the starting and ending linebreaks inside a quote when switching to Wysiwyg mode. (I told you, I never use Wysiwyg, except when I'm told it needs fixing...) I noticed that it didn't break any other linebreaks, so I looked into parse_bbc() and quickly remembered that I'd added a special regex just to get rid of extra whitespace. Disabling it fixed it. Then I figured, oh yeah it's because the Wysiwyg conversion shouldn't go through the same parsing steps as when actually showing the posts...
So I added a parameter to parse_bbc(), called $converting, which was set to true by the converter, and false otherwise, and it would then skip the line.
Then I thought, oh my that's a lot of parameters to deal with... And shouldn't it make more sense to ensure that it isn't called anytime there's non-standard parsing being done...? So I decided to instead go for an empty($parse_tags) test. It probably isn't perfect, but it should catch most of the occurrences, including the Wysiwyg one. So I did it that way.

Then I'm seeing the new commit, which adds an author ID parameter[1] to parse_bbc(), and really that made me smile.

So, thanks Pete for the coincidence and the smile ;)

I'm thinking, however, that we might want to instead provide a single extra parameter in the form of an array, where we could add our extra params like 'id_cache' and 'id_member' or whatever... Thus, no limits to what can be stored in the parser params, and they can in turn be transmitted to hooks so that plugins may intercept something that they target in particular.
What do you think...? Or is it too ugly of a solution?
Posted: January 19th, 2013, 02:25 PM

Code: [Select]
(empty($_POST['pin']) == empty($topic_info['is_pinned'])

I've always loved this kind of smart, elegant little trick ;)
 1. Also for the record, I think we both considered adding that parameter at least a couple of times in Wedge's lifetime, for various reasons!

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #516, on January 19th, 2013, 02:45 PM »
Great minds think alike :)

Actually, yeah, it would be better to pass things in through an array - in fact you could pass in everything other than message that way if you really wanted.

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #517, on January 19th, 2013, 02:55 PM »
Quote from Arantor on January 19th, 2013, 02:45 PM
Great minds think alike :)
But, on the same night? 8)
Quote
Actually, yeah, it would be better to pass things in through an array
It may not seem very 'clean', but it's certainly something that plugins might appreciate being able to do. Provided there are even hooks in parse_bbc() to deal with it, of course...
Quote
in fact you could pass in everything other than message that way if you really wanted.
What do you mean? (Or, to avoid having people think I can't speak English, "What did you have in mind?" :P)

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #518, on January 19th, 2013, 02:57 PM »
Well, right now we pass in message, whether to parse smileys[1], cache id, what tags to parse and lastly owner id.

We could convert all those other options into a single array, since there's places we don't use cache id, places we use cache id to denote signature and stuff like that - and not all places need an owner id, nor do all places need tags-to-parse lists.

Giving it a series of options does seem a bit cleaner.
 1. And I still want to remove print page from that!

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #519, on January 23rd, 2013, 12:44 PM »
That's the thing, $cache_id has always been used in a strange way... (Even re: original Aeva.)

So, the only thing that could stop us is performance. But I don't think passing an array has any impact whatsoever on performance... Does it?

Also, can I assume you're planning to remove everything related to packages, now? Your latest commit looks like it. I'm working on it right now and would like to know for instance if you want me to keep the package server code, or if you want to reuse it for plugins, or something like that...

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #520, on January 23rd, 2013, 02:04 PM »
Passing an array should not be significantly more expensive than passing a scalar variable.

And yes, stuff I haven't removed yet may still have some small use.

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #521, on January 23rd, 2013, 02:12 PM »
Okay, then... At least may I remove anything related to the one DB table you left, log_packages?

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #522, on January 23rd, 2013, 02:14 PM »
No because there is still code referencing it, and not just in the upgrader.

It will all be going soon enough!

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #523, on January 23rd, 2013, 02:18 PM »
Re: parse_bbc, ideally we'd get rid of everything but smileys (because these are pretty much in every function call), but I don't know what structure would work best for the cache ID... For instance, array('cache' => $id_msg, 'member' => $id_member) would be odd in a Display message parser, because that should most likely be array('id_msg' => $id_msg, ...) and parse_bbc should determine what elements are in the array and build the cache id from that... But it's slower, I guess.

So.. I don't really have any ideas to make it better right now. :-/

Re: log_packages, I can assure you I removed everything and it didn't take long... I'll double check.
Re: New revs - Public comments
« Reply #524, on January 23rd, 2013, 02:22 PM »
Yup... loadInstalledPackages() in Subs-Package uses it, and that's pretty much all. I just commented out the line in PackageGet() that makes use of it. As a reminder to write that part as well...

If I were you, I'd just make a backup somewhere of the package-related files, then remove EVERYTHING, and re-use that code on the side to build the plugin server stuff, and then commit it later. Right now, having all those 'package' references just sounds a bit odd.