Using regular expressions in a modification.xml...

Started by kriation, July 11, 2009, 03:37:30 PM

Previous topic - Next topic

kriation

Hi folks - I'm in the process of building a calendar modification, and I'd like to place a piece of code before the following segment in Calendar.php:

    updateStats('calendar');
}

// Returns true if this user is allowed to link the topic in question.
function calendarCanLink()
{


The regular expression I'm using in my modification.xml is the following:

\s*updateStats\(\'calendar\'\)\;\s*\}\s*\/\/\s*Returns true if this user is allowed to link the topic in question\.\s*


Any ideas on why when I upload the package to the forum, the test fails? I've validated that the expression is valid for the text that I'm looking for using a regular expression validator, but I definitely know that there are nuances in every validator (including the parser that's a part of Subs-Package.php).

I tried searching for the data without a regular expression as well (simply defining the text that I wanted) within the xml, and that test also failed.

As reference, I'm testing on a virgin SMF 1.1.9 instance, running on Apache on CentOS.

Any help would be much appreciated.

-- Kriation

[SiNaN]

Can you attach your XML modification file here?
Former SMF Core Developer | My Mods | SimplePortal

kriation

Quote from: [SiNaN] on July 16, 2009, 01:49:26 PM
Can you attach your XML modification file here?

Sure.... the following is the contents of the file (with the actual source code additions removed).

<?xml version="1.0"?>
<!DOCTYPE modification SYSTEM "http://www.simplemachines.org/xml/modification">

<modification xmlns="http://www.simplemachines.org/xml/modification" xmlns:smf="http://www.simplemachines.org/">
   
    <id>kriation:packageName</id>
    <version>0.1</version>

    <!-- Making changes to core SMF code -->
    <file name="$sourcedir/Subs.php">
        <operation>
            <search position="end" />
            <add><![CDATA[Source Code]]></add>
        </operation>
    </file>

    <file name="$sourcedir/Calendar.php">
        <operation>
            <search position="before" regexp="true"><![CDATA[/\s*updateStats\('calendar'\);\s*}\s*\/\/\s*Returns true if this user is allowed to link the topic in question./gx]]></search>
            <add><![CDATA[Source Code]]></add>
        </operation>
    </file>
</modification>

kriation

Just an update to those for those who are curious...

Through testing, I firmly believe that search/replace via regular expressions in the Package Manager is broken. My solution to this issue was to modify Subs-Package.php.

The change I made was with how the "loose" whitespace check worked. In the original code (found below), the test was to look for erroneous tabs using the character class for tabs (\t). Since tab usage is most certainly inconsistent across development platforms (some editors insert spaces, instead of a tab control character), this was effectively useless. The most disturbing factor is that whomever wrote Subs-Package clearly thought that '\t' would search for tabs and spaces, as reflected by their comment within the segment of code.


// Using 'loose', a random amount of tabs and spaces may be used.
                    if ($search['loose_whitespace'])
                        $actual_operation['searches'][$i]['preg_search'] = preg_replace('~[ \t]+~', '[ \t]+', $actual_operation['searches'][$i]['preg_search']);
                }


I replaced the '\t' with an '\s', which defines the regex character class for white space characters (including spaces, tabs, and line breaks). With the replacement of the character class, the search and replace worked wonderfully.

Is there any reason why '\t' was used instead of '\s', and what's the process to report a bug, and/or present a patch for SMF?

For reference, this change was made on a 1.1.10 instance (which was previously 1.1.9) on CentOS running Apache with PHP v5.1.6.

Arantor

Apart from that \t should in theory preserve whitespace better, using regexp is actually a bad idea since it means you can't readily have an uninstaller (meaning you either have to make users uninstall manually, or write a regexp in reverse to uninstall)

You're not recommended to use regexp mode for that reason.

Topic marked as solved due to not being followed up inside a month.

kriation

Quote from: Arantor on August 16, 2009, 10:25:36 PM
Apart from that \t should in theory preserve whitespace better, using regexp is actually a bad idea since it means you can't readily have an uninstaller (meaning you either have to make users uninstall manually, or write a regexp in reverse to uninstall)

You're not recommended to use regexp mode for that reason.

Topic marked as solved due to not being followed up inside a month.

That's an interesting way to 'solve' a topic...

The way I see it is that Package Management in 1.x is very broken. I understand that the community is working on 2.x, and that there will be a migration to it at some point, but we need to improve developer support for building packages for 1.x. There are a couple of problems (regarding search/replace specifically) that I've run into in building my own packages that I think should be addressed.

1) Developers should have two ways to provide search/replace subjects within their modification XMLs; one being the kludgey pass-in-text method that we have which basically takes the text subject and creates a regular expression via preg_quote (referenced in Subs-Package) and the second being the facility to pass in a regular expression to provide a precise search/replace. Regarding the uninstall 'issue', I agree with the statement that in using regular expressions, it makes uninstalling a package difficult. Why not enforce the presence of an uninstall script when using regular expressions as arguments, during installation? I think we can safely assume that a newbie won't be using regular expressions to produce a quick/dirty package, and will take the time to build an uninstall script...

2) Fix loose_whitespace - Without replacing the '\t' with '\s', this logic is pointless. All we're doing is looking for multiple tab characters... I understand using '\t' to preserve white space, but the code doesn't even look at non-tab characters making it quite difficult to pass in text since various OSs and editors deal with tabs/spaces differently.

Do package developers only use Windows with Wordpad? :)

Thoughts?

Arantor

Well, as it hadn't been replied to in almost a month I kinda figured the topic had dropped - and this way it was removed from the outstanding support issues. But as it isn't solved, you've done the right thing, reopened it and replied.

I'm not seeing an issue here. I build packages for 1.1 and haven't had a problem - but I don't use regexp type replacements. And I can use the same install script as uninstall script by simply listing it as <modification type="file" reverse="true">file.xml</modification>. Minimal effort required for uninstallation at all.

I'm still not sure I see how using a regular expression is easier than, say:
<search type="replace"><![CDATA[some stuff I want to change]]></search>
<add><![CDATA[what I want to change it to]]></add>


For the record I use Notepad++ and am well aware of the different between \t and \s, and have 10 mods to my name.

kriation

Quote from: Arantor on August 17, 2009, 03:22:21 PM
Well, as it hadn't been replied to in almost a month I kinda figured the topic had dropped - and this way it was removed from the outstanding support issues. But as it isn't solved, you've done the right thing, reopened it and replied.
Thanks. I figured it'd be worth continuing the discussion.

Quote from: Arantor on August 17, 2009, 03:22:21 PM
I'm not seeing an issue here. I build packages for 1.1 and haven't had a problem - but I don't use regexp type replacements. And I can use the same install script as uninstall script by simply listing it as <modification type="file" reverse="true">file.xml</modification>. Minimal effort required for uninstallation at all.

I'm still not sure I see how using a regular expression is easier than, say:
<search type="replace"><![CDATA[some stuff I want to change]]></search>
<add><![CDATA[what I want to change it to]]></add>


For the record I use Notepad++ and am well aware of the different between \t and \s, and have 10 mods to my name.

My real issue with search/replace is that it doesn't provide a consistent user experience across a wide range of development environments. All of my development is done on a Linux workstation with Vi, and it's clear that the search mechanism doesn't take into account the potential variation that exists when specifying search criteria without regular expressions. For example, since the modifications are presented in an XML format, my first inclination is to properly format my code. However, if I do, the search functionality breaks, because of a line feed/carriage return or space that isn't accounted for when creating the regular expression; even with loose_whitespace on, the search string is only looking for extra tab characters.

I honestly think this is an inherent flaw in how we create search strings in Sub-Package, especially with the perceived functionality that loose_whitespace is supposed to provide; it just doesn't work as the individual who wrote Subs-Package intended (a comment within Sub-Package states that with '\t', extra tabs and spaces will be dealt with, which is completely false).

I'm coming from the school that every developer should write code that someone else can pick up and understand. I hate downloading a package, having to full screen the file (forget about the 80 column constraint), and then attempting to understand improperly formatted code.

Arantor

I don't personally care how the code looks in the package at all, I just package to ensure it works on the base SMF setup - where everything is tab-indented.

I should note that despite the bulk of files being \n newlined, and all my mods having \r\n newlines doesn't cause a problem at all, and when I load the source files after modification (without regexp) I get the layout exactly as it should be.

I guess I'm not seeing the problem because I don't really see the necessity of using the regexp operation at all. :(

kriation

Quote from: Arantor on August 17, 2009, 04:06:55 PM
I don't personally care how the code looks in the package at all

If this is the general consensus of the SMF community, then I'll stop pursuing this topic because it's evidently not a moot point. Although, the fact that there exists documentation that establish Coding Guidelines, I'm hoping you're not part of the majority with this opinion...

Arantor

I didn't say it was the general consensus. That's why I specifically said: personally.

All I care about is that the code ends up in the final modified source file in line with the original code. Meaning that when I come to review it later - it fits in with the original code. So if you look at my mods, for example, in the actual package it's kinda messy. But compare the files before and after the modifications have been applied. Still messy?

kriation

Quote from: Arantor on August 17, 2009, 04:36:12 PM
I didn't say it was the general consensus. That's why I specifically said: personally.
My comment was questioning the general consensus agreeing with your opinion... not stating that your opinion was the opinion of the general consensus.

Quote from: Arantor on August 17, 2009, 04:36:12 PM
All I care about is that the code ends up in the final modified source file in line with the original code. Meaning that when I come to review it later - it fits in with the original code. So if you look at my mods, for example, in the actual package it's kinda messy. But compare the files before and after the modifications have been applied. Still messy?

Some of us like to keep our code neat regardless of where it is... the way I see it, a package is exactly that; a collection of code as a unit. If someone should open it up, it should be easy and clear to read and understand. The end result is also important, but my concern definitely is around the formatting of the source within the packages. Providing faculties to enable proper formatting of code should be something that's considered, whether it for the 1.x branch, or for 2.x.

I wouldn't consider this topic solved, as no solution has really come from our conversation, only a standstill in differing opinions. The 1.x Package Manager currently doesn't provide the flexibility in parsing to enable developers to write cleaner/easier to read code, and to satisfy the masses, I have to either 'messy' my code to fit the parsing engine, or write a mod to provide more flexibility in the parsing engine.

Arantor

Question: if you look at the modified source, do you expect it to fit in?

There isn't a best fit solution that keeps the mod clean and the resultant code completely in sync unless you want to write one that can actually figure out what indenting to keep.

Alternative solution: bundle the functionality up into functions, insert at end of script, then simply have a function call.

You reference the coding guidelines. They actually support what I'm saying: they provide examples of modifying DB queries in-situ - using indents in the code.

kriation

Quote from: Arantor on August 17, 2009, 05:09:47 PM
Question: if you look at the modified source, do you expect it to fit in?

There isn't a best fit solution that keeps the mod clean and the resultant code completely in sync unless you want to write one that can actually figure out what indenting to keep.
Forgive me if I'm coming across harsh, but I've been writing code professionally for a number of years, and the first thing that comes to mind when I start a new project is portability. I want to make sure I write code that someone else can pick up on day 1, and fully understand. When I starting writing my SMF package, I wanted the code within the package to be easily read, as well as the resulting code that's modified by my package. Based on my experience so far, the Package Manager in 1.x doesn't satisfy the first case at all, and barely satisfies the second.

Quote from: Arantor on August 17, 2009, 05:09:47 PM
Alternative solution: bundle the functionality up into functions, insert at end of script, then simply have a function call.
I've actually taken this approach since that's how I'm used to writing software anyway. The changes I've specified to make during installation are ones that facilitate the inclusion of my own PHP files, and the addition of functions within the core SMF code.

Quote from: Arantor on August 17, 2009, 05:09:47 PM
You reference the coding guidelines. They actually support what I'm saying: they provide examples of modifying DB queries in-situ - using indents in the code.

Right. For example, the following search fails with the default implementation of the package manager in 1.x.


<operation>
            <search position="after" whitespace="loose"><![CDATA[
            updateStats('calendar');
            }
            // Returns true if this user is allowed to link the topic in question.
            ]]></search>


The reason it fails is because there's whitespace that isn't a tab character between the opening bracket after CDATA and the text I'm searching for. The package manager simply can't handle this use case unless I specific the whitespace="loose" toggle, and change the '\t' to a '\s' within Subs-Package.

Arantor

Ah, yes, now I understand where you're coming from, and indeed I am actually a mere amateur programmer.

I can tell you now it won't change in 1.1.x at all unless it introduced a security vuln.

Question: does your change cause other mods to fail installation, even on an otherwise fresh install? Also, does this same thing happen in 2.0?

kriation

Quote from: Arantor on August 17, 2009, 06:13:44 PM
Ah, yes, now I understand where you're coming from, and indeed I am actually a mere amateur programmer.

I can tell you now it won't change in 1.1.x at all unless it introduced a security vuln.
Understood... initially, I was thinking about making a modification to support this use case, but it might not be worth it.

Quote from: Arantor on August 17, 2009, 06:13:44 PM
Question: does your change cause other mods to fail installation, even on an otherwise fresh install? Also, does this same thing happen in 2.0?
I'll test it on 2.x and report back.

[SiNaN]

Quote from: kriation on July 18, 2009, 02:27:30 PM
The change I made was with how the "loose" whitespace check worked. In the original code (found below), the test was to look for erroneous tabs using the character class for tabs (\t). Since tab usage is most certainly inconsistent across development platforms (some editors insert spaces, instead of a tab control character), this was effectively useless. The most disturbing factor is that whomever wrote Subs-Package clearly thought that '\t' would search for tabs and spaces, as reflected by their comment within the segment of code.


// Using 'loose', a random amount of tabs and spaces may be used.
                    if ($search['loose_whitespace'])
                        $actual_operation['searches'][$i]['preg_search'] = preg_replace('~[ \t]+~', '[ \t]+', $actual_operation['searches'][$i]['preg_search']);
                }


I replaced the '\t' with an '\s', which defines the regex character class for white space characters (including spaces, tabs, and line breaks). With the replacement of the character class, the search and replace worked wonderfully.

Is there any reason why '\t' was used instead of '\s', and what's the process to report a bug, and/or present a patch for SMF?

For reference, this change was made on a 1.1.10 instance (which was previously 1.1.9) on CentOS running Apache with PHP v5.1.6.

The regex in question includes spaces too. There is a space char just before \t in the regexes. \s is not used because that it will catch any whitespace char, including line breaks. That regex is intended to catch tabs and spaces as stated in the comment line.

Quote from: kriation on August 17, 2009, 05:56:22 PM
Right. For example, the following search fails with the default implementation of the package manager in 1.x.


<operation>
            <search position="after" whitespace="loose"><![CDATA[
            updateStats('calendar');
            }
            // Returns true if this user is allowed to link the topic in question.
            ]]></search>


The reason it fails is because there's whitespace that isn't a tab character between the opening bracket after CDATA and the text I'm searching for. The package manager simply can't handle this use case unless I specific the whitespace="loose" toggle, and change the '\t' to a '\s' within Subs-Package.

Not really. It fails because there is a linebreak between the curly bracket and the comment line. As I said above, whitespace="loose" won't be tolerant to linebreaks.

Also, if you are too much worried about the readability of your modification files, you could try boardmod type. You can find an example in the package SDK attached here.
Former SMF Core Developer | My Mods | SimplePortal

kriation

Quote from: [SiNaN] on August 25, 2009, 07:21:39 AM
The regex in question includes spaces too. There is a space char just before \t in the regexes. \s is not used because that it will catch any whitespace char, including line breaks. That regex is intended to catch tabs and spaces as stated in the comment line.

I actually realized that the space does exist the other day while looking at the code again. I apologize for stating that it doesn't earlier in this thread. I still feel that the search is too restrictive, and should account for '\r' and '\n'...

Quote from: [SiNaN] on August 25, 2009, 07:21:39 AM
Not really. It fails because there is a linebreak between the curly bracket and the comment line. As I said above, whitespace="loose" won't be tolerant to linebreaks.

Also, if you are too much worried about the readability of your modification files, you could try boardmod type. You can find an example in the package SDK attached here.
Even after adding a line break between the curly bracket and the comment line, the search still failed. Has anyone put together a unit test for testing the search functionality within the package manager for package developers?

You make it seem that being concerned about readability of source code is a fault... All developers should be concerned about the readability of their source code. I'll take a look at the boardmod type...

[SiNaN]

Quote from: kriation on August 25, 2009, 10:28:49 AM
Even after adding a line break between the curly bracket and the comment line, the search still failed. Has anyone put together a unit test for testing the search functionality within the package manager for package developers?

Can you attach the package you are using?

Quote from: kriation on August 25, 2009, 10:28:49 AM
You make it seem that being concerned about readability of source code is a fault... All developers should be concerned about the readability of their source code. I'll take a look at the boardmod type...

Readability of source code is important of course. But the only thing you can do with XML is using correct tabs. The rest is its nature and you can't do much about it.
Former SMF Core Developer | My Mods | SimplePortal

tmarques

Hi all,

I'm trying to get this simple mod to pass the tests but it just doesn't:

<?xml version="1.0"?>
<!DOCTYPE modification SYSTEM "http://www.simplemachines.org/xml/modification">
<modification xmlns="http://www.simplemachines.org/xml/modification" xmlns:smf="http://www.simplemachines.org/">
    <id>tmarques:Mod_Skin_2009_Metodo</id>
    <type>modification</type>
    <version>0.0.1</version>
    <file name="$sourcedir/Admin.php">
         <operation>
            <search position="before" regexp="true">
                <![CDATA[\t*// Mod Authors for a "ADD AFTER" on this line. Ensure you end your change with a comma. For example:]]></search>
            <add><![CDATA[
                'mdtheme' => array($txt['mods_cat_mdtheme']),
                 ]]></add>
         </operation>
     </file>
     <file name="$sourcedir/ManageSettings.php">
         <operation>
            <search position="before" regexp="true">
                <![CDATA[\t*// Mod authors, once again, if you have a whole section to add do it AFTER this line, and keep a comma at the end.]]></search>
            <add><![CDATA[
                'mdtheme' => 'ModifyMDThemeSettings',
                ]]></add>
         </operation>


I close the file and modification tags, all is fine structurally.
The problem is the first operation goes ok, if even barely, but the second doesn't at all. At first I couldn't get the first one to pass without using regexp, which is also strange. I was using just:


<search position="before" regexp="true">
                <![CDATA[// Mod Authors for a "ADD AFTER" on this line. Ensure you end your change with a comma. For example:]]></search>


Can someone please help me?

Best regards

Arantor

Try rewriting it thus:

<?xml version="1.0"?>
<!DOCTYPE modification SYSTEM "http://www.simplemachines.org/xml/modification">
<modification xmlns="http://www.simplemachines.org/xml/modification" xmlns:smf="http://www.simplemachines.org/">
    <id>tmarques:Mod_Skin_2009_Metodo</id>
    <type>modification</type>
    <version>0.0.1</version>
    <file name="$sourcedir/Admin.php">
         <operation>
            <search position="before"><![CDATA[// Mod Authors for a "ADD AFTER" on this line. Ensure you end your change with a comma. For example:]]></search>
            <add><![CDATA[
'mdtheme' => array($txt['mods_cat_mdtheme']),]]></add>
         </operation>
     </file>
     <file name="$sourcedir/ManageSettings.php">
         <operation>
            <search position="before"><![CDATA[// Mod authors, once again, if you have a whole section to add do it AFTER this line, and keep a comma at the end.]]></search>
            <add><![CDATA[
'mdtheme' => 'ModifyMDThemeSettings',]]></add>
         </operation>
     </file>
</modification>


Basically, if you're doing a position="before", you don't have to match the entire line, just enough to match its position, so you can completely discount the tab at the start of that block (and thus not use regexp)

Note that the ideal is to use tabs in the replacements (and not using tabs is a reason for not approving a mod), and I've replaced the leading spaces with two tabs, since IIRC that's the indentation necessary to keep it in style with the rest of the array.

tmarques

It doesn't work Arantor, now it all fails.
I'm remotely accessing the machine and am using Vim. I disabled expandtab, so it's just puting tabs there.
Any ideas? :(

Quote from: Arantor on September 30, 2009, 04:59:06 PM
Try rewriting it thus:

<?xml version="1.0"?>
<!DOCTYPE modification SYSTEM "http://www.simplemachines.org/xml/modification">
<modification xmlns="http://www.simplemachines.org/xml/modification" xmlns:smf="http://www.simplemachines.org/">
    <id>tmarques:Mod_Skin_2009_Metodo</id>
    <type>modification</type>
    <version>0.0.1</version>
    <file name="$sourcedir/Admin.php">
         <operation>
            <search position="before"><![CDATA[// Mod Authors for a "ADD AFTER" on this line. Ensure you end your change with a comma. For example:]]></search>
            <add><![CDATA[
'mdtheme' => array($txt['mods_cat_mdtheme']),]]></add>
         </operation>
     </file>
     <file name="$sourcedir/ManageSettings.php">
         <operation>
            <search position="before"><![CDATA[// Mod authors, once again, if you have a whole section to add do it AFTER this line, and keep a comma at the end.]]></search>
            <add><![CDATA[
'mdtheme' => 'ModifyMDThemeSettings',]]></add>
         </operation>
     </file>
</modification>


Basically, if you're doing a position="before", you don't have to match the entire line, just enough to match its position, so you can completely discount the tab at the start of that block (and thus not use regexp)

Note that the ideal is to use tabs in the replacements (and not using tabs is a reason for not approving a mod), and I've replaced the leading spaces with two tabs, since IIRC that's the indentation necessary to keep it in style with the rest of the array.

Arantor

Both operations fail? No reason they should do that.

tmarques

Quote from: Arantor on October 01, 2009, 04:22:30 PM
Both operations fail? No reason they should do that.

Yup. That's why I had said that I used the regular expression to make it work but only the first one did.
What's the consensus about using vim and editing the files on site? Could it be related?
I'm using SMF 2.0 RC1.2

Best regards

Arantor


tmarques

Quote from: Arantor on October 01, 2009, 08:28:27 PM
Does vim definitely save the tabs correctly?

I think so. At least it behaved as such.
Can you take a look at the file for me?
I'll try editing it with notepad but that really isn't how I'd like to proceed from now on, as I only have windows installed on a VM and usually do all development remotely. I still have some months of work in SMF ahead of me and it would be very nice if this worked better. Do you think I can help improve this package manager "problem"?

Best regards

Arantor


Dannii

Regex's shouldn't be used just for formatting like that... I think a more valid use case is for removing something from an array (like a menu item) if other things have been inserted into the array after the mod was installed.
"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."

tmarques

Quote from: Arantor on October 02, 2009, 05:25:27 AM
Can you attach that package here please?

Sure. I have a bit more code in the original package than the one I posted here, but I have formatted it with tabs either way. The full code is in attach.
I also had tried the boardmod format but it was even worse, allways telling me that no "Install operations defined in the package" or something like that.

Best regards

Arantor

Well, I note you're still using \t at the start - with what I suggested, take that out.

I would also suggest not using 2.0 RC1-1 in the package info file, as there are known issues with that (which is why RC1.2 is 1.2 not 1-2)

tmarques

Quote from: Arantor on October 02, 2009, 06:45:53 AM
Well, I note you're still using \t at the start - with what I suggested, take that out.

I would also suggest not using 2.0 RC1-1 in the package info file, as there are known issues with that (which is why RC1.2 is 1.2 not 1-2)

My bad, I forgot to remove the \t* on those. Did as you said right now but I'm still getting the same failed test on all of them. I also had some missing tabs in code to be added, also fixed that now.
See the attachment please.
Best regards

Arantor

In the actual file there is a line break between the end of the <search> and the start of the CDATA. As per XML, that's still part of the search string, because it's the contents of the search tag.

tmarques

Quote from: Arantor on October 02, 2009, 07:47:40 AM
In the actual file there is a line break between the end of the <search> and the start of the CDATA. As per XML, that's still part of the search string, because it's the contents of the search tag.

Thanks Arantor, that fixed it. This completely screws up formatting of the XML, which is very unfortunate, since it becomes a bit of a hindrance to maintain mods.
Are there any plans to fix this?

Arantor

Not really because it's not fixable by definition. It is following the schema precisely; find the contents of <search> and add in the content for <add>. <![CDATA[ as per XML specification simply says that the data in it should not be parsed by the XML parser, so you don't have entity issues. I would note that this is inherited from the original SGML specification which is now decades old.

How would you change it without it not being XML?

The fact is, unless you either have a modified XML parser (meaning that those of us that use XML generation tools will have to modify them too) or you don't use XML at all, it is unavoidable.

Dannii

"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."

tmarques

Quote from: Arantor on October 02, 2009, 08:40:59 AM
Not really because it's not fixable by definition. It is following the schema precisely; find the contents of <search> and add in the content for <add>. <![CDATA[ as per XML specification simply says that the data in it should not be parsed by the XML parser, so you don't have entity issues. I would note that this is inherited from the original SGML specification which is now decades old.

How would you change it without it not being XML?

The fact is, unless you either have a modified XML parser (meaning that those of us that use XML generation tools will have to modify them too) or you don't use XML at all, it is unavoidable.

I see, I have no idea about XML, that was why I was asking. I was suggesting that we do as we like inside the CDATA, and then SMF would take care of it, independent of the leading spaces or tabs, something like that. I don't know much about parsing, so I don't know if this is something feasible to implement, I'm just saying after looking at the parsing code that uses just tabs.
If there isn't any easy fix, guess I have to live with it.
Thanks for everything.

Best regards

tmarques

Quote from: Dannii on October 02, 2009, 08:41:42 AM
How does it screw up the formatting??

I was referring to readability of the code after. The mod was a lot "cleaner" when I had the code I indented my way but that didn't work.

Arantor

Do you mean the readability of the mod's XML file, or the readability of the code injected into the source files? The two are quite different, the latter is vastly easier to deal with since you as mod author have the ability to manage that.

Dannii

Quote from: tmarques on October 02, 2009, 09:13:32 AM
I was referring to readability of the code after. The mod was a lot "cleaner" when I had the code I indented my way but that didn't work.
Didn't look clean to me... you have the search and add tags at the same level as the operations etc.
"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."

tmarques

Quote from: Dannii on October 02, 2009, 10:43:21 AM
Quote from: tmarques on October 02, 2009, 09:13:32 AM
I was referring to readability of the code after. The mod was a lot "cleaner" when I had the code I indented my way but that didn't work.
Didn't look clean to me... you have the search and add tags at the same level as the operations etc.

I know, it was kind of misaligned but was more readable to me. I still have a lot to learn when it comes to indent code, especially on something I haven't had much experience with :/

Dannii

I suggest making sure they work perfectly first ;) It's only an xml file... it's not meant to be read by humans.
"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."

Arantor

Making it readable is often desirable but unfortunately the standard doesn't really allow for that, as it is an XML issue. It simply isn't designed to be human readable given what it's doing.

Dannii

And if needed you could always add spaces in the tags etc: <add                ><![CDATA[
"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: