Extra <br/> Break Tags

[WizGeek]

Pulled a fork on May 15 and noticed there now are unnecessary <br/> tags generated between markup elements. I don't know when this was introduced since the January release, but I'd guess somewhere between March and now.

---

Problem Rendering Snippet: ...<div class="main"><h1>Header Level 1</h1> <br/>This text follows<br/><div class="footer">...

---

Old Rendering Snippet: ...<div class="main"><h1>Header Level 1</h1>This text follows<br/><div class="footer">...

[amolenaar]

I would suggest the text should be in paragraph tags. Use br's al over the place makes it undoable to apply proper style on it.

...<div class="main"><h1>Header Level 1</h1>
<p>This text follows</p><div class="footer">...

[newmilieu]

Absolutely agree with both. I recently tried to run a jQuery script across a test page (to extract results, apply some styling, etc), and I basically stopped when I saw the line break tags - jQuery doesn't play nicely with poorly formatted html :P

[stanio]

I would suggest the text should be in paragraph tags. Use br's al over the place makes it undoable to apply proper style on it.

Boy, this wiki text parser seems complicated... I would really like to contribute for a more complete solution to this problem. Please have a look at the following patch (or better yet - try it!). It is much in a draft shape; you'll need to build skipping tests as I haven't looked into updating the relevant ones: mvn clean install -Dmaven.test.skip=true. There are some edge cases to clear out, but it appears to work most of the time for me.

I would like to get feedback whether the proposed changes appear on the right track and any comments/directions to drive it to the end:

# HG changeset patch
# User Stanimir Stamenkov <stanio@yahoo.com>
# Date 1334577012 -10800
# Node ID 20e5a6bb498e4113171267b31e3b3b75639d20b9
# Parent  ffe7936ac27ccba1adf2afd47ebf157ab717b1c8
Parse paragraph blocks

diff --git a/src/fitnesse/wiki/PageData.java b/src/fitnesse/wiki/PageData.java
--- a/src/fitnesse/wiki/PageData.java
+++ b/src/fitnesse/wiki/PageData.java
@@ -3,12 +3,10 @@
 package fitnesse.wiki;

 import fitnesse.responders.run.ExecutionLog;
 import static fitnesse.wiki.PageType.*;
 import fitnesse.wikitext.parser.*;
-import fitnesse.wikitext.parser.HtmlTranslator;
-import fitnesse.wikitext.parser.Paths;
 import util.Clock;
 import util.Maybe;
 import util.StringUtil;

 import java.io.Serializable;
@@ -96,11 +94,11 @@
     contentSyntaxTree = data.contentSyntaxTree;
     parsingPage = data.parsingPage;
   }

   public void initializeAttributes() {
-    if (!isErrorLogsPage()) { 
+    if (!isErrorLogsPage()) {
       properties.set(PropertyEDIT, Boolean.toString(true));
       properties.set(PropertyADD_CHILD, Boolean.toString(true));
       properties.set(PropertyPROPERTIES, Boolean.toString(true));
       properties.set(PropertyREFACTOR, Boolean.toString(true));
     }
@@ -224,11 +222,12 @@
     }

     private void parsePageContent() {
         if (contentSyntaxTree == null) {
             parsingPage = new ParsingPage(new WikiSourcePage(wikiPage));
-            contentSyntaxTree = Parser.make(parsingPage, getContent()).parse();
+            contentSyntaxTree = Parser.make(parsingPage, getContent(),
+                    new SymbolProvider(SymbolProvider.wikiParsingProvider).add(new Paragraph())).parse();
         }
     }

   public void setLiterals(List<String> literals) {}

diff --git a/src/fitnesse/wikitext/parser/Matcher.java b/src/fitnesse/wikitext/parser/Matcher.java
--- a/src/fitnesse/wikitext/parser/Matcher.java
+++ b/src/fitnesse/wikitext/parser/Matcher.java
@@ -8,11 +8,11 @@

     private interface ScanMatch {
         Maybe<Integer> match(ScanString input, int offset);
     }

-    private static final ArrayList<Character> defaultList = new ArrayList<Character>();
+    private static final ArrayList<Character> defaultList = new ArrayList<Character>(1);
     static {
         defaultList.add('\0');
     }

     private ArrayList<ScanMatch> matches = new ArrayList<ScanMatch>();
@@ -51,11 +51,11 @@
         return this;
     }

     public Matcher string(final String delimiter) {
         if (firsts == null) {
-            firsts = new ArrayList<Character>();
+            firsts = new ArrayList<Character>(1);
             firsts.add(delimiter.charAt(0));
         }
         matches.add(new ScanMatch() {
             public Maybe<Integer> match(ScanString input, int offset) {
                 return input.matches(delimiter, offset) ? new Maybe<Integer>(delimiter.length()) : Maybe.noInteger;
@@ -109,11 +109,11 @@
         return this;
     }

     public Matcher repeat(final char delimiter) {
         if (firsts == null) {
-            firsts = new ArrayList<Character>();
+            firsts = new ArrayList<Character>(1);
             firsts.add(delimiter);
         }
         matches.add(new ScanMatch() {
             public Maybe<Integer> match(ScanString input, int offset) {
                 int size = 0;
diff --git a/src/fitnesse/wikitext/parser/Paragraph.java b/src/fitnesse/wikitext/parser/Paragraph.java
new file mode 100644
--- /dev/null
+++ b/src/fitnesse/wikitext/parser/Paragraph.java
@@ -0,0 +1,177 @@
+package fitnesse.wikitext.parser;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+import fitnesse.html.HtmlTag;
+
+import util.Maybe;
+
+public class Paragraph extends SymbolType implements Rule, Translation {
+
+    private static final Symbol LineBreak =
+            new Symbol(new SymbolType("LineBreak")
+                    .htmlTranslation(new HtmlBuilder("br")));
+
+    private static SymbolType[] defaultBlockTypes;
+    static {
+        SymbolType[] types = {
+            new Table(),
+            new HashTable(),
+            new HeaderLine(),
+            new Collapsible(),
+            new Contents(),
+            SymbolType.CenterLine,
+            //new Define(),
+            //new Help(),
+
+            //new Include(),
+            //SymbolType.Meta,
+            //SymbolType.NoteLine,
+            //new PlainTextTable(),
+            //See.symbolType,
+
+            new HorizontalRule(),
+            SymbolType.UnorderedList,
+            SymbolType.OrderedList,
+            Comment.symbolType,
+            //SymbolType.Newline,
+            Preformat.symbolType
+        };
+        defaultBlockTypes = types;
+    }
+
+    /*private*/ static List<Character> matcherFirsts;
+    static {
+        List<Character> list = new ArrayList<Character>(130);
+        list.add('\0');
+        //for (char c = 'a'; c <= 'z'; c++) list.add(c);
+        //for (char c = 'A'; c <= 'Z'; c++) list.add(c);
+        //for (char c = '0'; c <= '9'; c++) list.add(c);
+        list.add('\t');
+        //list.add('\n');
+        for (char c = 33; c <= 127; c++) list.add(c);
+        matcherFirsts = list;
+    }
+
+    /*private*/ SymbolType[] blockTypes;
+
+    /*private*/ boolean inlineContext = false;
+
+    private SymbolType[] blockTypesAndNewline;
+
+    private ParaMatcher matcher = new ParaMatcher();
+
+    private Paragraph(SymbolType[] blockTypes) {
+        super("Paragraph");
+        this.blockTypes = blockTypes;
+        blockTypesAndNewline = Arrays.copyOf(blockTypes, blockTypes.length + 1);
+        blockTypesAndNewline[blockTypes.length] = SymbolType.Newline;
+        wikiMatcher(matcher);
+        wikiRule(this);
+        htmlTranslation(this);
+    }
+
+    public Paragraph() {
+        this(defaultBlockTypes);
+    }
+
+    public Paragraph(List<SymbolType> blockTypes) {
+        this(blockTypes.toArray(new SymbolType[blockTypes.size()]));
+    }
+
+    @Override
+    public Maybe<Symbol> parse(Symbol current, Parser parser) {
+        if (inlineContext) {
+            return Symbol.nothing;
+        }
+        inlineContext = true;
+
+        Symbol textRun = parser.parseToEnds(0,
+                SymbolProvider.wikiParsingProvider, blockTypesAndNewline);
+        boolean lineBreak = false;
+        while (textRun.getChildren().size() > 0) {
+            if (lineBreak) {
+                current.add(LineBreak);
+                lineBreak = false;
+            }
+            current.add(textRun);
+
+            Symbol next = parser.peek();
+            if (next.isType(SymbolType.Newline)) {
+                parser.moveNext(1);
+                lineBreak = true;
+            }
+            textRun = parser.parseToEnds(0,
+                    SymbolProvider.wikiParsingProvider, blockTypesAndNewline);
+        }
+
+        inlineContext = false;
+        Symbol result = isWhitespaceOnly(current)
+                        ? new Symbol(SymbolType.SymbolList)
+                        : current;
+        return new Maybe<Symbol>(result);
+    }
+
+    private boolean isWhitespaceOnly(Symbol content) {
+        for (Symbol child : content.getChildren()) {
+            if (child.isType(SymbolType.SymbolList)) {
+                if (!isWhitespaceOnly(child)) {
+                    return false;
+                }
+            } else if (!(child.isType(SymbolType.Whitespace)
+                         || child.isType(SymbolType.Newline))) {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    @Override
+    public String toTarget(Translator translator, Symbol symbol) {
+        boolean raw = inlineContext;
+        inlineContext = true;
+        StringBuffer buf = new StringBuffer();
+        for (Symbol child : symbol.getChildren()) {
+            buf.append(translator.translate(child));
+        }
+        String slim = buf.toString().trim();
+        if (raw) {
+            return slim;
+        }
+        inlineContext = false;
+        return new HtmlTag("p", slim).html();
+    }
+
+
+    private class ParaMatcher extends Matcher {
+
+        ParaMatcher() {
+            //super();
+        }
+
+        @Override
+        public List<Character> getFirsts() {
+            return matcherFirsts;
+        }
+
+        @Override
+        public Maybe<Integer> makeMatch(ScanString input) {
+            if (inlineContext || !input.startsLine(0)) {
+                return Maybe.noInteger;
+            }
+
+            for (SymbolType type : blockTypes) {
+                SymbolMatch match = type.makeMatch(input);
+                if (match.isMatch()) {
+                    return Maybe.noInteger;
+                }
+            }
+            return new Maybe<Integer>(0);
+        }
+
+    } // class ParaMatcher
+
+
+} // class Paragraph
diff --git a/src/fitnesse/wikitext/parser/Parser.java b/src/fitnesse/wikitext/parser/Parser.java
--- a/src/fitnesse/wikitext/parser/Parser.java
+++ b/src/fitnesse/wikitext/parser/Parser.java
@@ -10,11 +10,11 @@
     private static final ArrayList<Symbol> emptySymbols = new ArrayList<Symbol>();

     public static Parser make(WikiPage page, String input) {
         return make(new ParsingPage(new WikiSourcePage(page)), input);
     }
-    
+
     public static Parser make(ParsingPage currentPage, String input) {
         return make(currentPage, input, SymbolProvider.wikiParsingProvider);
     }

     public static Parser make(ParsingPage currentPage, String input, SymbolProvider provider) {
@@ -60,10 +60,19 @@
             tokens.add(current);
         }
         return tokens;
     }

+    public Symbol peek() {
+        List<Symbol> lookAhead = peek(1);
+        return lookAhead.get(0);
+    }
+
+    public List<Symbol> peek(int lookAheadSize) {
+        return scanner.peek(lookAheadSize, new ParseSpecification().provider(specification));
+    }
+
     public List<Symbol> peek(SymbolType[] types) {
         List<Symbol> lookAhead = scanner.peek(types.length, new ParseSpecification().provider(specification));
         if (lookAhead.size() != types.length) return emptySymbols;
         for (int i = 0; i < lookAhead.size(); i++) {
             if (!lookAhead.get(i).isType(types[i])) return emptySymbols;
@@ -105,11 +114,11 @@
     }

     public Symbol parseToIgnoreFirstWithSymbols(SymbolType ignore, SymbolProvider provider) {
         return parse(new ParseSpecification().ignoreFirst(ignore).terminator(ignore).provider(provider));
     }
-    
+
     public Symbol parseTo(SymbolType terminator) {
         return parseTo(terminator, 0);
     }

     public Symbol parseTo(SymbolType terminator, int priority) {
diff --git a/src/fitnesse/wikitext/parser/ScanString.java b/src/fitnesse/wikitext/parser/ScanString.java
--- a/src/fitnesse/wikitext/parser/ScanString.java
+++ b/src/fitnesse/wikitext/parser/ScanString.java
@@ -97,6 +97,15 @@
         for (char c: content.toCharArray()) {
             if (!Character.isLetterOrDigit(c) && c != '_' && c != '.') return false;
         }
         return true;
     }
+
+    @Override
+    public String toString() {
+        if (input == null || offset > input.length()) {
+            return super.toString();
+        }
+        return input.substring(offset);
+    }
+
 }
diff --git a/src/fitnesse/wikitext/parser/Symbol.java b/src/fitnesse/wikitext/parser/Symbol.java
--- a/src/fitnesse/wikitext/parser/Symbol.java
+++ b/src/fitnesse/wikitext/parser/Symbol.java
@@ -27,11 +27,11 @@
     public void setContent(String content) { this.content = content; }

     public Symbol childAt(int index) { return getChildren().get(index); }
     public Symbol lastChild() { return childAt(getChildren().size() - 1); }
     public List<Symbol> getChildren() { return children; }
-    
+
     public Symbol addToFront(Symbol child) {
         ArrayList<Symbol> newChildren = new ArrayList<Symbol>();
         newChildren.add(child);
         newChildren.addAll(children);
         children = newChildren;
@@ -110,7 +110,11 @@
                 : type == Literal.symbolType ? SymbolType.CloseLiteral
                 : type == Comment.symbolType ? SymbolType.Newline
                 : SymbolType.Empty;
     }

+    @Override
+    public String toString() {
+        return String.valueOf(getType()) + '(' + getContent() + getChildren() + ')';
+    }

 }
diff --git a/src/fitnesse/wikitext/parser/SymbolProvider.java b/src/fitnesse/wikitext/parser/SymbolProvider.java
--- a/src/fitnesse/wikitext/parser/SymbolProvider.java
+++ b/src/fitnesse/wikitext/parser/SymbolProvider.java
@@ -8,26 +8,27 @@
             Literal.symbolType, Preformat.symbolType, Link.symbolType, Path.symbolType, WikiWord.symbolType,
             SymbolType.Newline, SymbolType.Whitespace
     });

     public static final SymbolProvider wikiParsingProvider = new SymbolProvider( new SymbolType[] {
+            //new Paragraph(),
             Link.symbolType, new Table(), SymbolType.EndCell,
             new HashTable(),  new HeaderLine(), Literal.symbolType, new Collapsible(),
             new AnchorName(), new Contents(), SymbolType.CenterLine, new Define(), new Help(),
             new Include(), SymbolType.Meta, SymbolType.NoteLine, Path.symbolType, new PlainTextTable(),
             See.symbolType, SymbolType.Style, new LastModified(), Image.symbolType,
-            new Today(), SymbolType.Delta, 
+            new Today(), SymbolType.Delta,
             new HorizontalRule(), SymbolType.CloseLiteral, SymbolType.Strike,
             Alias.symbolType, SymbolType.UnorderedList, SymbolType.OrderedList, Comment.symbolType, SymbolType.Whitespace, SymbolType.CloseCollapsible,
             SymbolType.Newline, SymbolType.Colon, SymbolType.Comma,
             Evaluator.symbolType, SymbolType.CloseEvaluator, Variable.symbolType, Preformat.symbolType,
             SymbolType.ClosePreformat, SymbolType.OpenParenthesis, SymbolType.OpenBrace, SymbolType.OpenBracket,
             SymbolType.CloseParenthesis, SymbolType.CloseBrace, SymbolType.ClosePlainTextTable, SymbolType.CloseBracket, SymbolType.CloseLiteral,
             SymbolType.Bold,
             SymbolType.Italic, SymbolType.Strike, new AnchorReference(), WikiWord.symbolType, SymbolType.EMail, SymbolType.Text,
     });
-    
+
     public static final SymbolProvider aliasLinkProvider = new SymbolProvider(
             new SymbolType[] {SymbolType.CloseBracket, Evaluator.symbolType, Literal.symbolType, Variable.symbolType});

     public static final SymbolProvider linkTargetProvider = new SymbolProvider(
             new SymbolType[] {Literal.symbolType, Variable.symbolType});
@@ -64,11 +65,11 @@
     public void addTypes(Iterable<SymbolType> types) {
         for (SymbolType symbolType: types) {
             add(symbolType);
         }
     }
-    
+
     public SymbolProvider add(SymbolType symbolType) {
         if (matchesFor(symbolType)) return this;
         symbolTypes.add(symbolType);
         for (Matcher matcher: symbolType.getWikiMatchers()) {
             for (char first: matcher.getFirsts()) {
@@ -86,11 +87,11 @@

     public void addMatcher(Matchable matcher) {
         ArrayList<Matchable> defaults = currentDispatch.get(defaultMatch);
         defaults.add(matcher);
     }
-    
+
     public boolean matchesFor(SymbolType type) {
         return symbolTypes.contains(type);
     }

     public SymbolMatch findMatch(ScanString input, MatchableFilter filter) {
diff --git a/src/fitnesse/wikitext/parser/SymbolType.java b/src/fitnesse/wikitext/parser/SymbolType.java
--- a/src/fitnesse/wikitext/parser/SymbolType.java
+++ b/src/fitnesse/wikitext/parser/SymbolType.java
@@ -4,11 +4,11 @@

 import java.util.ArrayList;
 import java.util.List;

 public class SymbolType implements Matchable {
-    
+
     public static final SymbolType Bold = new SymbolType("Bold")
             .wikiMatcher(new Matcher().string("'''"))
             .wikiRule(new EqualPairRule())
             .htmlTranslation(new HtmlBuilder("b").body(0).inline());
     public static final SymbolType CenterLine = new SymbolType("CenterLine")
@@ -53,12 +53,13 @@
     public static final SymbolType Meta = new SymbolType("Meta")
             .wikiMatcher(new Matcher().string("!meta"))
             .wikiRule(new LineRule())
             .htmlTranslation(new HtmlBuilder("span").body(0).attribute("class", "meta").inline());
     public static final SymbolType Newline = new SymbolType("Newline")
-            .wikiMatcher(new Matcher().string("\n"))
-            .htmlTranslation(new HtmlBuilder("br").inline());
+            .wikiMatcher(new Matcher().string("\n"));
+    //public static final SymbolType EmptyLine = new SymbolType("EmptyLine")
+    //        .wikiMatcher(new Matcher().startLine().ignoreWhitespace().string("\n"));
     public static final SymbolType NoteLine = new SymbolType("NoteLine")
             .wikiMatcher(new Matcher().string("!note"))
             .wikiRule(new LineRule())
             .htmlTranslation(new HtmlBuilder("span").body(0).attribute("class", "note").inline());
     public static final SymbolType OpenBrace = new SymbolType("OpenBrace")
@@ -87,21 +88,28 @@
             .wikiMatcher(new Matcher().startLine().whitespace().string("*"))
             .wikiRule(new ListRule())
             .htmlTranslation(new ListBuilder("ul"));
     public static final SymbolType Whitespace = new SymbolType("Whitespace")
             .wikiMatcher(new Matcher().whitespace());
-    
+
     private String name;
     private ArrayList<Matcher> wikiMatchers =  new ArrayList<Matcher>();
     private Rule wikiRule = null;
     private Translation htmlTranslation = null;
+    private boolean blockLevel = false;

     public SymbolType(String name) { this.name = name; }

+    public SymbolType(String name, boolean blockLevel) {
+        this.name = name;
+        this.blockLevel = blockLevel;
+    }
+
     public List<Matcher> getWikiMatchers() { return wikiMatchers; }
     public Rule getWikiRule() { return wikiRule; }
     public Translation getHtmlTranslation() { return htmlTranslation; }
+    public boolean isBlockLevel() { return blockLevel; }

     public SymbolType wikiMatcher(Matcher value) {
         wikiMatchers.add(value);
         return this;
     }

[stanio]

Perhaps one will need to place to following CSS rule in the style sheet, also:

table {
    margin: 1em 0;
}

The following rule in fitnesse_straight.css becomes obsolete:

/** Page content (article) styles **/
h1+br, h2+br, h3+br, h4+br, h5+br, h6+br {
    display: none;
}

[amolenaar]

Wow! This looks really good already.

You implemented toString() on a few classes. I suppose that's only for debugging?

@jediwhale: Is this the right approach?

[stanio]

Yep, I've added toString() just to ease debugging... I was about to post an updated cleaned up patch, but trying it more I'm really not sure it is feasible, currently.

I think we should first resolve issue #76 and the proposed fix from conradmueller is o.k. for starters.

[jediwhale]

I'll take a look - won't be for a few days as very busy right now.

On 2012-04-17 00:32, Arjan Molenaar wrote:

Wow! This looks really good already.

You implemented toString() on a few classes. I suppose that's only for debugging?

@jediwhale: Is this the right approach?

---

Reply to this email directly or view it on GitHub: https://github.com/unclebob/fitnesse/issues/30#issuecomment-5169118

Cheers, Mike Stockdale

/fit/Sharp http://fitsharp.github.com Syterra Software Inc. http://www.syterra.com

[jediwhale]

Fixed the original issue. Should open another one for adding paragraph tags - I think this needs some wider discussion before we change the way wiki markup is rendered.