[JLINE-730] Support for comments in DefaultParser

[snuyanzin]

The PR makes it possible to set multiline comment start and end.
Also it allows to specify several one-line comments as several engines support several one-line comments

fixes #730

[mattirn]

There are some minor things I would like to be fixed before merge.

1. 'one line comments' and 'multi-line comments' are better known as 'line comments' and 'block comments', see for example Comment (computer programming). We should use these names to construct method and variable names.
2. The setters of line comment delimiters should be renamed as
   setLineCommentDelims( ... ) and lineCommentDelims( ... )
3. IMHO we should not set individually block comment delimiters. We should have methods that set block comment delimiters as:
   setBlockCommentDelims(new BlockCommentDelims(<start>, <end>))
   The class BlockCommentDelims we can implement as an inner class in DefaultParser. BlockCommentDelims constructor should throw InvalidArgumentException if either of the comment delimiters is null/empty or if delimiters are equal.
4. We do not need the boolean field eofOnUnclosedBlockComment in DefaultParser. If the block comment delimiters are set we will always throw EofError on not closed block comments.
5. The method isLineStartsWith( ... ) should be renamed as isCommentDelim( ... ).
6. The method isOneLineCommentStarted( ... ) should be renamed as isLineCommentDelim( ... ) and its scope should be private.
7. The methods isMultiLineCommentStarted( ... ) and isMultiLineCommentClosed( ... ) can be removed. We can call directly the method isCommentDelim( ... )

[snuyanzin]

Hi @mattirn
thanks for the feedback.
I've made changes based on it.

Besides that I also noticed a failing test case like hello/*comment*/world produced only one word ['helloworld'] while in fact there should be ['hello', 'world']. I fixed that and added this to tests.

[snuyanzin]

I extracted this into a method as now this code block is used in 2 places.
In case you have a better method name suggestion I would consider that

[mattirn]

1. BlockCommentDelims constructor should throw InvalidArgumentException if either of start or end argument is null/empty or if they are equal:
   throw new InvalidArgumentException("Bad block comment delimiter!")

2. I would rename also private field lineComments as lineCommentDelims .
   

   jline3/reader/src/main/java/org/jline/reader/impl/DefaultParser.java

   Line 59 in 654855d

   private String[] lineComments = null;

3. On missing block comment delimiter the error message should be "Missing closing block comment delimiter"
   

   jline3/reader/src/main/java/org/jline/reader/impl/DefaultParser.java

   Line 375 in 654855d

   throw new EOFError(-1, -1, "Missing closing block comment",

Please, could you squash the commits in a single commit when finished.

Thanks.

[snuyanzin]

ok, thanks, done

[mattirn]

1. IllegalArgumentException should thrown also when start.isEmpty() || end.isEmpty()
   

   jline3/reader/src/main/java/org/jline/reader/impl/DefaultParser.java

   Line 34 in 6241811

   if (start == null || end == null || start.equals(end)) {

2. same test executed twice
   

   jline3/reader/src/test/java/org/jline/reader/completer/DefaultParserTest.java

   Line 99 in 6241811

   parser.setBlockCommentDelims(new DefaultParser.BlockCommentDelims("/*", null));

   
   and
   

   jline3/reader/src/test/java/org/jline/reader/completer/DefaultParserTest.java

   Line 104 in 6241811

   parser.setBlockCommentDelims(new DefaultParser.BlockCommentDelims("/*", null));

[snuyanzin]

Sorry, had to check it before pushing...
Fixed it.
Please let me know if I can squash commits

[mattirn]

NP. I will do some more testing next week.... I let you know if I find something.

[snuyanzin]

Probably also need to check that block start and end differ from one line comments and from brackets and quotes

[mattirn]

Probably also need to check that block start and end differ from one line comments and from brackets and quotes

Partially true... brackets can be used as block comment delimiters if they do not have any other use in CLI. I would not add any other validity checks for comment delimiters. IMHO checks would just make the code more complicate to follow without bringing much more value for implementation. Anyway I guess that the 'standard' comment delimiters (like //, #, /*, */, ...) will be mostly adopted in Jline CLIs

[mattirn]

I think we should throw EOFError also for missing opening block comment:

            if (startBlockCommentMissing) {
                throw new EOFError(-1, -1, "Missing opening block comment delimiter",
                        "missing: " + blockCommentStart);
            }

Please, change also the line

jline3/reader/src/main/java/org/jline/reader/impl/DefaultParser.java

Line 379 in 9c9d638

throw new EOFError(-1, -1, "Missing closing block comment delimiter",

to

                throw new EOFError(-1, -1, "Missing closing block comment delimiter",
                        "add: " + blockCommentEnd);