SMF 1.1 Package Manager changes - Question for the developers

Started by Rudolf, December 17, 2006, 06:03:53 PM

Previous topic - Next topic

Rudolf

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
I will update all my mods in the next few weeks. Thanks for your patience.

SVG-Collapse (you need an SVG compliant browser)

Compuart

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.
Hendrik Jan Visser
Former Lead Developer & Co-founder www.simplemachines.org
Personal Signature:
Realitynet.nl -> ExpeditieRobinson.net / PekingExpress.org / WieIsDeMol.Com

Rudolf

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.
I will update all my mods in the next few weeks. Thanks for your patience.

SVG-Collapse (you need an SVG compliant browser)

Oldiesmann


Rudolf

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?
I will update all my mods in the next few weeks. Thanks for your patience.

SVG-Collapse (you need an SVG compliant browser)

Compuart

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.
Hendrik Jan Visser
Former Lead Developer & Co-founder www.simplemachines.org
Personal Signature:
Realitynet.nl -> ExpeditieRobinson.net / PekingExpress.org / WieIsDeMol.Com

Rudolf

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:
 

       
  • modifying the DTD file - 1 line of code
  • modifying the xmlArray class - max 10 lines of code
  • modifying the parseModification function - max 20 lines of code

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.
I will update all my mods in the next few weeks. Thanks for your patience.

SVG-Collapse (you need an SVG compliant browser)

Compuart

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.
Hendrik Jan Visser
Former Lead Developer & Co-founder www.simplemachines.org
Personal Signature:
Realitynet.nl -> ExpeditieRobinson.net / PekingExpress.org / WieIsDeMol.Com

Dannii

I'll have to look into using regex search patterns.. might make it much easier to make compatible mods :)
"Never imagine yourself not to be otherwise than what it might appear to others that what you were or might have been was not otherwise than what you had been would have appeared to them to be otherwise."

Advertisement: