Skip to content

Commit

Permalink
correctly formatting each line output by the help command
Browse files Browse the repository at this point in the history
  • Loading branch information
Andy Berry committed Sep 28, 2015
1 parent abaf8ae commit e53884d
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 4 deletions.
45 changes: 44 additions & 1 deletion brjs-core-tests/src/test/java/org/bladerunnerjs/plugin/commands/core/HelpCommandTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,16 @@ public class HelpCommandTest extends SpecTest
HelpCommand helpCommand;
MockCommandPlugin command1;
MockCommandPlugin command2;
MockCommandPlugin command3;
MockCommandPlugin command4;

@Before
public void initTestObjects() throws Exception
{
command1 = new MockCommandPlugin("command1", "Command #1 description.", "command-usage", "Command #1 help.");
command2 = new MockCommandPlugin("command2", "Command #2 description.", "command-usage", "Command #2 help.");
command3 = new MockCommandPlugin("command3", "Command #3 description.", "command-usage", "Command #3 help.\ncommand #3 line 2");
command4 = new MockCommandPlugin("command4", "Command #4 description.", "command-usage", "Command #4 help.\n badly formatted line\n \tand a tabbed line");
}

@Test
Expand All @@ -35,13 +39,15 @@ public void exceptionIsThrownIfThereAreTooManyArguments() throws Exception
@Test
public void helpCommandListsAllPossibleCommands() throws Exception
{
given(brjs).hasCommandPlugins(command1, command2)
given(brjs).hasCommandPlugins(command1, command2, command3, command4)
.and(brjs).hasBeenCreated();
when(brjs).runCommand("help");
then(logging).containsConsoleText(
"Possible commands:",
" command1 : Command #1 description. ",
" command2 : Command #2 description. ",
" command3 : Command #3 description. ",
" command4 : Command #4 description. ",
" -----",
" help : Prints this list of commands ",
" version : Displays the BladeRunnerJS version ",
Expand Down Expand Up @@ -79,6 +85,43 @@ public void helpForSpecificCommandShowsUsage() throws Exception
" Command #1 help.");
}

@Test
public void multilineHelpCommandIsCorrectlyFormatted() throws Exception
{
given(brjs).hasCommandPlugins(command3)
.and(brjs).hasBeenCreated();
when(brjs).runCommand("help", "command3");
then(logging).containsConsoleText(
"Description:",
" Command #3 description.",
"",
"Usage:",
" brjs command3 command-usage",
"",
"Help:",
" Command #3 help.",
" command #3 line 2");
}

@Test
public void multilineHelpCommandIsCorrectlyFormattedWhenItContainsSomeWhitespace() throws Exception
{
given(brjs).hasCommandPlugins(command4)
.and(brjs).hasBeenCreated();
when(brjs).runCommand("help", "command4");
then(logging).containsConsoleText(
"Description:",
" Command #4 description.",
"",
"Usage:",
" brjs command4 command-usage",
"",
"Help:",
" Command #4 help.",
" badly formatted line",
" \tand a tabbed line");
}

@Ignore
@Test
public void commandIsAutomaticallyLoaded() throws Exception
Expand Down
21 changes: 18 additions & 3 deletions brjs-core/src/main/java/org/bladerunnerjs/plugin/commands/core/HelpCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.util.List;

import org.apache.commons.lang3.StringUtils;
import org.bladerunnerjs.api.BRJS;
import org.bladerunnerjs.api.logging.Logger;
import org.bladerunnerjs.api.model.exception.command.CommandArgumentsException;
Expand Down Expand Up @@ -104,13 +105,27 @@ private void getHelpForSpecificCommand(String commandName) throws CommandArgumen
logger.println("");

logger.println("Help:");
logger.println( getFormattedHelpMessage(command) );
}

private String getFormattedHelpMessage(CommandPlugin command)
{
String commandHelp = command.getCommandHelp();
if (commandHelp.length() > 0 && !Character.isWhitespace(commandHelp.charAt(0))) {
commandHelp = " "+commandHelp;
StringBuilder formattedHelp = new StringBuilder();
for (String line : StringUtils.split(commandHelp, "\n")) {
formattedHelp.append( formatHelpMessageLine(line, 2) + "\n" );

This comment has been minimized.

Sign in to view

Copy link
@dchambers

dchambers Sep 28, 2015

Contributor

I think this line should just be:

formattedHelp.append("  " + line + "\n" );

where you modify JSAPArgsParsingCommandPlugin to strip the indent that JSAP automatically adds.
}
logger.println(commandHelp);
return formattedHelp.toString();
}

private String formatHelpMessageLine(String line, int expectedWhitespaceCount) {

This comment has been minimized.

Sign in to view

Copy link
@dchambers

dchambers Sep 28, 2015

Contributor

AFAICT, this method will break the indentation of commands that don't extend JSAPArgsParsingCommandPlugin. For example, if my command outputs this help message:

A
  B
    C

then we would expect this to be displayed like this in the terminal:

Help:
  A
    B
      C

whereas it will actually be displayed like this:

Help:
  A
  B
    C

This comment has been minimized.

Sign in to view

Copy link
@andy-berry-dev

andy-berry-dev Sep 29, 2015

Member

The formatHelpMessageLine only ensures that there is at least 2 characters at the start of line, nothing else. JSAP output is already indented with the correct number of spaces so no extra whitespace is added. I've added a test to verify this though and checked that theformatHelpMessageLine doesn't have any affect on a JSAP command's output.

This comment has been minimized.

Sign in to view

Copy link
@dchambers

dchambers Sep 29, 2015

Contributor

Morning @andyberry88,

I'm not talking about JSAP output being wrong. I'm talking about people not using JSAP, as in my example above.

This comment has been minimized.

Sign in to view

Copy link
@andy-berry-dev

andy-berry-dev Sep 29, 2015

Member

The same goes for commands that don't use JSAP. See multilineHelpCommandIsCorrectlyFormattedWhenItContainsSomeWhitespace. If the help command output contains enough whitespace then none will be added, otherwise we'll add 2 characters so each line is indented by at least 2 spaces. If the command author, whether using JSAP or not, wants to be a bit more elaborate with the output by adding extra spacing or tabs then they will be kept in the output.

Let's discuss this after the standup, I might not fully understand what you're meaning.

This comment has been minimized.

Sign in to view

Copy link
@andy-berry-dev

andy-berry-dev Sep 30, 2015

Member

After talking with @dchambers it looks like the above indenting example is an issue. We decided that the JSAP args parsing command should strip off it's first 2 whitespace characters so the help command can add them back in for every command's help output.
I've made this change in the latest commit in the PR and also added another test for the above example.
int lineWhitespace = 0;
while (lineWhitespace < line.length() && line.charAt(lineWhitespace) == ' ') {
lineWhitespace++;
}
return StringUtils.repeat(' ', Math.max(0, expectedWhitespaceCount - lineWhitespace)) + line;
}

public String getHelpMessageFormatString()
{
int commandNameSize = getLongestCommandName() + 5;
Expand Down

0 comments on commit e53884d

Please sign in to comment.