Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/java/org/aesh/command/impl/Executions.java
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ static <CI extends CommandInvocation> List<Execution<CI>> buildExecution(List<Pa
if (config == null) {
throw new IllegalArgumentException("Invalid " + pl.line());
}
config.setArgument(pl.line().trim());
config.setArgument(pl.firstWord().word());
state = State.NEED_OPERATOR;
break;
}
Expand Down
8 changes: 7 additions & 1 deletion src/main/java/org/aesh/parser/LineParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,13 @@ else if (parseCurlyAndSquareBrackets && (c == CURLY_END || c == PARENTHESIS_END)
handleCurlyEnd(c);
}
else if (haveEscape) {
builder.append(BACK_SLASH);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment. I would love ;-), to have the 2 parsing operations be merged in order to avoid to duplicate fixes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should probably look at trying to refactor that method so it could be inlined...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MMarus , I will try to spend some time (once your fixed is merged) to refactor the 2 methods.

Copy link
Contributor Author

@MMarus MMarus Feb 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have incorporated the changes you suggested into the PR. However, I didn't refactor the 2 methods in order to keep the PR cleaner. I am not sure why the github ignores changes I made here and still requests a change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems that the \ shouldn't be removed.

Copy link
Contributor Author

@MMarus MMarus Feb 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understand the previous comments the addition of \ should be replaced by the lines 201-207. Which add the \ when it is not escaping an operator:

if (!isQuoted()
  && (currentOperator = matchesOperators(operators, text, index)) != OperatorType.NONE) {
    // Do not add the \ that was a way to escape an operator.
} else {
    builder.append(BACK_SLASH);
}

Otherwise the \ would be inserted 2 times in case it is not escaping an operator.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MMarus , I read too quickly and thought that it was the other parse method. Forget about this comment.

//Escaping an operator?
if (!isQuoted()
&& (currentOperator = matchesOperators(operators, text, index)) != OperatorType.NONE) {
// Do not add the \ that was a way to escape an operator.
} else {
builder.append(BACK_SLASH);
}
builder.append(c);
haveEscape = false;
}
Expand Down
7 changes: 7 additions & 0 deletions src/main/java/org/aesh/parser/ParsedLine.java
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,13 @@ public ParsedWord lastWord() {
return words().get(words.size()-1);
}

public ParsedWord firstWord() {
if(words.size() > 0 )
return words.get(0);
else
return new ParsedWord("", 0);
}

public int size() {
return words().size();
}
Expand Down
17 changes: 17 additions & 0 deletions src/test/java/org/aesh/parser/LineParserTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -474,4 +474,21 @@ public void testOperatorParsing() {
assertEquals("", lines.get(1).selectedWord().word());
}

@Test
public void testParseEscapedCharacters() {
assertEquals("mkdir", parseLine("mkdir He\\|lo").get(0).word());
assertEquals("Try to escape |", "He|lo", parseLine("mkdir He\\|lo").get(1).word());
assertEquals("Try to escape ;", "He;lo", parseLine("mkdir He\\;lo").get(1).word());
assertEquals("Try to escape \\","He\\lo", parseLine("mkdir He\\\\lo").get(1).word());
assertEquals("Try to escape normal char","He\\lo", parseLine("mkdir He\\lo").get(1).word());
assertEquals("Try to escape normal char","He\\-o", parseLine("mkdir He\\-o").get(1).word());
}

List<ParsedWord> parseLine(String line) {
LineParser lineParser = new LineParser();
EnumSet<OperatorType> operators = EnumSet.allOf(OperatorType.class);

return lineParser.parseLine(line, -1, false, operators).get(0).words();
}

}
35 changes: 35 additions & 0 deletions src/test/java/org/aesh/parser/ParsedLineTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package org.aesh.parser;

import org.aesh.command.operator.OperatorType;
import org.junit.Test;

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

import static org.junit.Assert.*;

/**
* Created by Marek Marusic <mmarusic@redhat.com> on 2/19/18.
*/
public class ParsedLineTest {
@Test
public void firstWordFromEmptyLine() throws Exception {
List<ParsedWord> words = new ArrayList<>();
ParsedLine pl = new ParsedLine("", words, -1,
0, 0, ParserStatus.OK, "", OperatorType.NONE);
assertEquals(pl.firstWord().word(), "");
}

@Test
public void firstWordFromLineWithWords() throws Exception {
List<ParsedWord> words = new ArrayList<>();
words.add(new ParsedWord("command", 0));
words.add(new ParsedWord("line", 1));
words.add(new ParsedWord("text", 2));

ParsedLine pl = new ParsedLine("command line text", words, -1,
0, 0, ParserStatus.OK, "", OperatorType.NONE);
assertEquals(pl.firstWord().word(), "command");
}

}