Item8903: INCLUDE will return entire topic if pattern does not match
Priority: Normal
Current State: Closed
Released In: 1.0.10, 1.1.0
Target Release: patch
Applies To: Engine
Component:
Branches:
If you do
%INCLUDE{"WebHome" pattern="^.*?(WILL NOT MATCH).*"}%
and your
WebHome does not contain the text "WILL NOT MATCH" it will include the entire
WebHome
Might have to do with:
http://trac.foswiki.org/browser/branches/Release01x00/core/lib/Foswiki.pm#L1987
--
JohannesHammersen - 13 Apr 2010
On 1.1, the problem is in
Foswiki::Macros::INCLUDE::applyPatternToIncludedText() :
$text =~ s/$pattern/$1/is;
I suggest that should be changed to:
# The trailing () ensures that $1 is defined if the pattern matches
# for the case where $pattern does not contain (...)
if ( $text =~ /$pattern()/is ) {
$text = $1;
}
else {
$text = ''; # but see "Warning if pattern does not match" below
}
That will allow us to dispense with the leading and trailing
.*
in
INCLUDE's
$pattern
most (if not all) of the time, whilst existing patterns that
do have leading and trailing
.*
will continue to work.
There could potentially be uses of
INCLUDE with $pattern that might break. All those that I have though of rely on undocumented behaviour:
- Existing, undocumented behaviour: If the pattern does not match then the whole topic is returned
- Existing, undocumented behaviour: If the pattern omits leading (or trailing)
.*
, then the portion of the topic before the pattern (or after, if trailing .*
is omitted) will also be included.
I view both of these aspects of the existing behaviour as bugs.
Warning if pattern does not match
INCLUDE warns if the topic does not exist. I think it should also warn if the pattern does not match.
warn="off"
should disable the warning.
Kenneth - I added you to the "waiting for" list so that you are notified of this patch
--
MichaelTempest - 05 Jul 2010
Both $pattern in
SEARCH and
INCLUDE are very goofy and geeky.
I have not get come to terms with what works and what does not work.
But I have noticed and taken advantage of using ^ as the start of the $pattern in
SEARCH. But I never managed to get anything to work if it did not end with a .* and that is so strange.
Especially in
SEARCH with multiple="on" the scope of $pattern should be the one line of text found, and then I never understood why I always had to end with .*
So I am all for improving this. But watch out for the beginning of the string. We do not want to break using ^ to match beginning of line.
--
KennethLavrsen - 15 Jul 2010
I changed
INCLUDE to return blank if the pattern does not match in 1.1.
TODO: Make
INCLUDE warn if the pattern does not match
--
MichaelTempest - 16 Jul 2010
Including a part of another topic and returning nothing is a feature.
It could be that you want to include user comments from another topic and if there are none you want none.
So the warning - which is a good tool for someone trying to get a regex to work - will be a problem.
INCLUDE has the
warn
parameter where the warning is either on or off or a predefined message. So
INCLUDE should only warn when warn is not off. And we should not break the current way warn works. So maybe we already have the feature?
There are two different cases. Either the included topic does not exist. Or the topic exists but the pattern does not match.
I am not sure how warn works today when the pattern does not match. I would assume it just returns the entire topic and does not warn as this report says which is clearly a bug. So I would say it is OK to simply let the current warning feature work.
Maybe in 1.2 we can add a warn="debug" feature that can be more specific telling the application builder what is goind on. We could use same feature for
SEARCH.
--
KennethLavrsen - 17 Jul 2010
The warning for pattern-does-not-match will work the same as for topic-does-not-exist
--
MichaelTempest - 17 Jul 2010
It looks like this isn't working on trunk.foswiki.org :/
%INCLUDE{"Sandbox.WebHome" pattern="^.*?(WILL NOT MATCH Oh PURPLE ewe).*"}%
No permission to view Sandbox
--
MichaelTempest - 20 Jul 2010
It seems there was a problem with the updating trunk.foswiki.org. Now it works
I cannot see a good way to add a warning now without changing the interface to the IncludeHandlers. I doubt that would be a good idea after a feature freeze. I raised
Item9385 so that the need for a warning is not lost. For 1.1,
INCLUDE will return blank (empty string) if the pattern does not match.
--
MichaelTempest - 24 Jul 2010
I am back-porting the change to 1.0.x branch, so this change will also go into 1.0.10
--
MichaelTempest - 01 Aug 2010