Simple Machines Community Forum

Customizing SMF => SMF Coding Discussion => Topic started by: Rudolf on December 17, 2006, 06:03:53 PM

Title: SMF 1.1 Package Manager changes - Question for the developers
Post by: Rudolf on December 17, 2006, 06:03:53 PM
In Subs-Packages.php on Line 1554

// Finally, we're doing some replacements.
$working_data = preg_replace('~' . $actual_operation['searches'][$i]['preg_search'] . '~s', $actual_operation['searches'][$i]['preg_replace'], $working_data, 1);


Can any of the developers explain to me why the limit (1)??
It was a feature I actually used in my mods.
Now I have to create an operation for every occurrence of a text I want to replace multiple times. And it is not easy, because I have to use large chunks of text around it to find a unique one.
I really don't see the advantage of the limit, and don't see the lack of it as a bug.

It was a feature that made the package manager really powerful. Now it's all crippled.  :'(
is there any chance you consider reverting it as it was - without the limit? Or at least create an option for it?
Like using
Code (xml) Select

<operation count="_value_">
<search>
</search>
<add>
</add>
</operation>

where count is an optional attribute of int type and defaulting to 1.
So when it's -1 it replaces all matches
Title: Re: SMF 1.1 Package Manager changes - Question for the developers
Post by: Compuart on December 17, 2006, 06:50:40 PM
I can't imagine this was ever a feature. It could be considered a bug if a replacement would happen more than once. The modication system requires (or at least strongly suggests) to use a unique string for both the search and replace string, so that uninstalling the modification will not fail.

The thing that was actually a feature but didn't work properly in 1.1 RC3, was multiple searches, which should now work properly in 1.1.
Title: Re: SMF 1.1 Package Manager changes - Question for the developers
Post by: Rudolf on December 17, 2006, 07:20:48 PM
Yes I saw that multiple search feature. It seems to work.
Yes the search string should be a unique string, but all too often I have to replace/add after the same unique string in a file.
Here's a concrete example.
I was making a package to modify the Pots.template.php.
In it there are two strings:
var textFields = ["subject", "message", "icon", "guestname", "email", "evtitle", "question", "topic"];
and
var textFields = ["subject", "message", "guestname", "evtitle", "question"];

I need to add the "description" value into the array.
Before 1.1 I would simply write an operation like this:

<operation>
   <search position="before"><![CDATA["subject",]]></search>
   <add><![CDATA[" description",]]></search>
</operation>


And would work perfectly, including the uninstall.
Because in this case "subject", is a unique string that appears two times, and it just happens that I need to perform the same operation on both of them. Believe me, it was a great feature, even if it wasn't planned. I really see no point removing it.

The previous example was rather simple.
Check this out (Xml.template.php):

<operation>
<search position="before"><![CDATA[<modified><![CDATA[', empty($context['message']['modified']['time']) ? '' : '« <i>' . $txt[211] . ': ' . $context['message']['modified']['time'] . ' ' . $txt[525] . ' ' . $context['message']['modified']['name'] . '</i> »', ']]]]>><![CDATA[</modified>
<subject><![CDATA[', $context['message']['subject'], ']]]]>><![CDATA[</subject>]]></search>
<add><![CDATA[
<description><![CDATA[', $context['message']['description'], ']]]]>><![CDATA[</description>]]></add>
</operation>

I had to do this because SMF was not replacing a second occurrence of the same line, so I had to extend it.
Title: Re: SMF 1.1 Package Manager changes - Question for the developers
Post by: Oldiesmann on December 18, 2006, 12:06:40 PM
I've never known that to be a feature.
Title: Re: SMF 1.1 Package Manager changes - Question for the developers
Post by: Rudolf on December 18, 2006, 12:24:31 PM
It seems no one knows about it.
Anyway it worked fine.
So how about changing it back as it was, and calling it a new feature?
Title: Re: SMF 1.1 Package Manager changes - Question for the developers
Post by: Compuart on December 18, 2006, 02:36:50 PM
I see your point in wanting this feature. However allowing multiple non-unique search patterns as a feature is imho begging for problems when it comes to uninstalling modifications. The best chance for a modification to work both ways is to make sure both search and replace patterns are always unique.
Title: Re: SMF 1.1 Package Manager changes - Question for the developers
Post by: Rudolf on December 18, 2006, 03:17:25 PM
Quote from: Compuart on December 18, 2006, 02:36:50 PM
I see your point in wanting this feature. However allowing multiple non-unique search patterns as a feature is imho begging for problems when it comes to uninstalling modifications. The best chance for a modification to work both ways is to make sure both search and replace patterns are always unique.

Maybe I wasn't clear enough.
I have already used this starting back when SMF was in 1.0 beta. And it always worked since then, the reverse installation worked as expected.

The only time it would be dangerous is when you use regular expressions. But I still have to see a mod that used regular expressions for searching, and anyway you're constricted to write an uninstall operation for it.
Now, maybe with this new multiple searches the reverse operation could cause problems, I don't know. I'll do some testing on them when I get a bit of free time.

If it turns out that not having a limit could cause problems, could you at least modify the Package Manager the way I showed in the first post?
  It wouldn't take too much:
 
I'm willing to make the modifications and test, if you are willing at least to take a look at it when it's done.
My point is: putting the 1 limit is certainly beneficial for ensuring the stability, but excluding an option that was/is highly useful is quite annoying IMO. I mean in my idealistic world mods are maybe by people who know what they are doing. Unfortunately this is not always so. But shutting the door for those who know what they are doing is sad. A little back door, carefully hidden for advanced uses it's never bad.
Title: Re: SMF 1.1 Package Manager changes - Question for the developers
Post by: Compuart on December 20, 2006, 03:11:48 PM
To be honoust, I'm not very keen on changing it, as it's bound to go wrong when using multiple search keys (which is something that was a documented feature that was fixed in 1.1 final). Adding a feature of which you know in advance that it might cause problems for uninstalling modifications, is not wise in my humble opinion.

On another note, the most stable modifications are those that contain only unique search values and unique replace values. And even then, it could still happen that one modification prevents uninstalling the other, but at least it gives the best chances of a modification successfully (un)installing.
Title: Re: SMF 1.1 Package Manager changes - Question for the developers
Post by: Dannii on December 20, 2006, 05:19:16 PM
I'll have to look into using regex search patterns.. might make it much easier to make compatible mods :)