Priority: Low
Current State: Closed
Released In: 2.0.0
Target Release: major
EditingShorthand documents tables as "Use %VBAR% or | to add | characters in tables." It does not mention using \| to escape a column.
The following form causes a silent failure in the logs, when it attempts to find a handler for
type 40
, because the columns have all been shifted left due to the active escape. Note however the table renders correctly, ignoring the escape.
Name |
Type |
Size |
Values |
Description |
Attributes |
Default |
Something \ |
text |
40 |
|
|
|
|
The cause is in
Foswiki::Table::Parser
, commenting out the protection of \| fixes the Form processing.
@@ -184,7 +184,7 @@ sub parse {
if ( length($line) ) {
- $line =~ s/\\\|/\007/g; # protect \| from split
+ #$line =~ s/\\\|/\007/g; # protect \| from split
# Expand comments again after we split
my @cols =
map { _rewrite( $_, \@comments ) }
--
GeorgeClark - 26 Apr 2015
Foswiki 1.1.9 has the same issue, with a completely different table parser. So definitely low priority.
--
GeorgeClark - 26 Apr 2015
It doesn't mention
\|
because I'm not sure it is consistently supported.
should have
three columns.
Right, I disabled the escape in core and the
TablesContrib. That change needs a proposal.
--
Main.CrawfordCurrie - 20 Jun 2015 - 07:59
This breaks the
FormDef unit tests which expects the escaped separator.
Module Failure summary
FormDefTests has 2 unexpected results (of 10):
The test embeds a regex %SEARCH macro inside a table. Without the ability to escape the | or condition, I'm not sure just killing this "feature" is the right solution. It's been in trunk since 2011.
Item11040 and
distro:848b64bc59c5 I tried other ways of encoding the vbar to get it into the regex, but I've not come up with a solution.
Although, also this seems to have been slipped in as part of the
MongoDB work.
Hm This escape is also documented in
DataForms
Macros in the initial values of a form definition get expanded when the form definition is loaded.
- If you want to use a | character in the initial values field, you have to precede it with a backslash, thus: \|.
- ...
And a "git blame" traces that 1st bullet back to the initial import back in 2009. Although reading that literally, the only place the \| escape should be active is in a
DataForm initial values field.
--
GeorgeClark - 22 Jun 2015
I believe that the root cause is how the Foswiki::Form goes about parsing a table. In normal rendering, the %SEARCH would be expanded before the table is parsed in render, so there is no escaping necessary. But Foswiki::Form parses the table before any macro expansion has taken place.
I have a fix. Expand the % variables in Foswiki::Form before parsing the table. It requires some test changes because of the earlier macro expansion, but I don't believe that they are of consequence.
(Deleted this fix. not correct)
`
--
GeorgeClark - 22 Jun 2015
Foswiki::Form 1.1.x used it's own table parse code that escaped \| Foswiki::Form 1.2.x has been changed to use the core table parser which does NOT escape \|. So there are 3 issues here
- Foswiki 1.2 Form parser must have the \| escape for the values field.
- For this particular bug, escapes should not be active in any other column
- Table parsing should never use \| as an escape.
And my proposed solution above is not good, because early expansion of macros within form definitions will have performance implications.
--
GeorgeClark - 22 Jun 2015
I have a fix that doesn't change rendering order, and limits \| pipe escapes to just Forms.
diff --git a/core/lib/Foswiki/Tables/Parser.pm b/core/lib/Foswiki/Tables/Parser.pm
index d213369..2105cb7 100644
--- a/core/lib/Foswiki/Tables/Parser.pm
+++ b/core/lib/Foswiki/Tables/Parser.pm
@@ -100,6 +100,8 @@ a table (i.e. not verbatim or literal lines).
sub parse {
my ( $text, $dispatch ) = @_;
+ my $procform = ( (caller)[0] eq 'Foswiki::Form' );
+
my $in_table = 0;
my %scope = ( verbatim => 0, literal => 0, include => 0 );
my $openRow;
@@ -186,29 +188,44 @@ sub parse {
if ( length($line) ) {
- # See Item13385 for why this is commented out
- #$line =~ s/\\\|/\007/g; # protect \| from split
-
# Expand comments again after we split
my @cols =
map { _rewrite( $_, \@comments ) }
- map { s/\007/|/g; $_ }
split( /\|/, $line, -1 );
# Note use of LIMIT=-1 on the split so we don't lose
# empty columns
- foreach my $col (@cols) {
- my ( $prec, $text, $postc, $ish ) = split_cell($col);
- if ($ish) {
- print STDERR "TH '$prec', '$text', '$postc'\n"
+ my $rowlen = scalar @cols;
+ for ( my $i = 0 ; $i < $rowlen ; $i++ ) {
+ if ( $procform
+ && $i == 3
+ && ( substr( $cols[$i], -1 ) eq '\\' )
+ && $i < $rowlen )
+ {
+# Form definitions allow use of \| escapes in the initial values colunn - column 4
+# So this code removes the "splits" from within the initial values
+# But only when processing a form. See Item13385
+ print STDERR "Merging Form values column.\n"
if TRACE;
- &$dispatch( 'th', $prec, $text, $postc );
+ chop $cols[$i];
+ $cols[$i] .= '|' . splice( @cols, $i + 1, 1 );
+ $rowlen--;
+ redo;
}
else {
- print STDERR "TD '$prec', '$text', '$postc'\n"
- if TRACE;
- &$dispatch( 'td', $prec, $text, $postc );
+ my ( $prec, $text, $postc, $ish ) =
+ split_cell( $cols[$i] );
+ if ($ish) {
+ print STDERR "TH '$prec', '$text', '$postc'\n"
+ if TRACE;
+ &$dispatch( 'th', $prec, $text, $postc );
+ }
+ else {
+ print STDERR "TD '$prec', '$text', '$postc'\n"
+ if TRACE;
+ &$dispatch( 'td', $prec, $text, $postc );
+ }
}
}
}
--
GeorgeClark - 22 Jun 2015
Crawford. Fix checked in. It passed the unit tests, please set this to Waiting for Release, if the fix looks okay. Thanks.
--
GeorgeClark - 22 Jun 2015