Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added the option to specify a result file with --move VIEW #2075

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ Parameter descriptions:
-l, --language=<language>
Select the language of the submissions (default: java). See subcommands below.
-M, --mode=<{RUN, VIEW, RUN_AND_VIEW}>
The mode of JPlag: either only run analysis, only open the viewer, or do both (default: null)
The mode of JPlag. If VIEW is chosen, you can specify a result file to display with either the positional argument, '--new', '--old' or '-r'. Make
sure to only specify one if you do that. One of: RUN, VIEW, RUN_AND_VIEW (default: null)
-n, --shown-comparisons=<shownComparisons>
The maximum number of comparisons that will be shown in the generated report, if set to -1 all comparisons will be shown (default: 500)
-new, --new=<newDirectories>[,<newDirectories>...]
Expand Down
2 changes: 1 addition & 1 deletion cli/src/main/java/de/jplag/cli/CLI.java
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public void executeCli() throws ExitException, IOException {

switch (this.inputHandler.getCliOptions().mode) {
case RUN -> runJPlag();
case VIEW -> runViewer(null);
case VIEW -> runViewer(this.inputHandler.getFileForViewMode());
case RUN_AND_VIEW -> runViewer(runJPlag());
}
}
Expand Down
3 changes: 2 additions & 1 deletion cli/src/main/java/de/jplag/cli/options/CliOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ public class CliOptions implements Runnable {
"--result-file"}, description = "Name of the file in which the comparison results will be stored (default: ${DEFAULT-VALUE}). Missing .zip endings will be automatically added.")
public String resultFile = "results";

@Option(names = {"-M", "--mode"}, description = "The mode of JPlag. One of: ${COMPLETION-CANDIDATES} (default: ${DEFAULT_VALUE})")
@Option(names = {"-M",
"--mode"}, description = "The mode of JPlag. If VIEW is chosen, you can specify a result file to display with either the positional argument, '--new', '--old' or '-r'. Make sure to only specify one if you do that. One of: ${COMPLETION-CANDIDATES} (default: ${DEFAULT_VALUE})")
Comment on lines +56 to +57
Copy link
Member

Choose a reason for hiding this comment

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

I was wondering whether we should simplify this and not tell the users that there are multiple options. Just document one, but leave the functionality for all others in.

Copy link
Member

@tsaglam tsaglam Jan 16, 2025

Choose a reason for hiding this comment

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

Suggested change
"--mode"}, description = "The mode of JPlag. If VIEW is chosen, you can specify a result file to display with either the positional argument, '--new', '--old' or '-r'. Make sure to only specify one if you do that. One of: ${COMPLETION-CANDIDATES} (default: ${DEFAULT_VALUE})")
"--mode"}, description = "The mode of JPlag. One of: ${COMPLETION-CANDIDATES} (default: ${DEFAULT_VALUE}). If VIEW is chosen, you can optionally specify a path to an existing report.")

Copy link
Member

Choose a reason for hiding this comment

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

And for subm dir argument add a short sentence: if mode is view, you specify path report here.

public JPlagMode mode = JPlagMode.RUN_AND_VIEW;

@Option(names = {"--normalize"}, description = "Activate the normalization of tokens. Supported for languages: Java, C++.")
Expand Down
24 changes: 24 additions & 0 deletions cli/src/main/java/de/jplag/cli/picocli/CliInputHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import java.io.File;
import java.security.SecureRandom;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
Expand All @@ -29,6 +30,7 @@
public class CliInputHandler {
private static final String OPTION_LIST_HEADING = "Parameter descriptions: ";

private static final String AMBIGUOUS_VIEW_FILE = "There are multiple files selected for '--mode VIEW', Please make sure to only specify one.";
TwoOfTwelve marked this conversation as resolved.
Show resolved Hide resolved
private static final String UNKNOWN_LANGUAGE_EXCEPTION = "Language %s does not exists. Available languages are: %s";
private static final String IMPOSSIBLE_EXCEPTION = "This should not have happened."
+ " Please create an issue on github (https://github.com/jplag/JPlag/issues) with the entire output.";
Expand Down Expand Up @@ -169,4 +171,26 @@ private String generateDescription() {
var randomDescription = DESCRIPTIONS[RANDOM.nextInt(DESCRIPTIONS.length)];
return String.format(DESCRIPTION_PATTERN, randomDescription, CREDITS);
}

/**
* Returns the file to display when using --move VIEW. The result can be null, if no file was selected
* @return The file to show
* @throws CliException If multiple options would be valid
*/
public File getFileForViewMode() throws CliException {
List<File> validOptions = new ArrayList<>(List.of(this.options.rootDirectory));

validOptions.addAll(List.of(this.options.newDirectories));
validOptions.addAll(List.of(this.options.oldDirectories));

if (this.parseResult.hasMatchedOption('r')) {
TwoOfTwelve marked this conversation as resolved.
Show resolved Hide resolved
validOptions.add(new File(this.options.resultFile));
}

return switch (validOptions.size()) {
case 0 -> null;
case 1 -> validOptions.getFirst();
default -> throw new CliException(AMBIGUOUS_VIEW_FILE);
};
}
}
59 changes: 59 additions & 0 deletions cli/src/test/java/de/jplag/cli/ModeTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package de.jplag.cli;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrowsExactly;

import java.io.File;
import java.io.IOException;

import org.junit.jupiter.api.Test;

import de.jplag.cli.picocli.CliInputHandler;
import de.jplag.cli.test.CliArgument;
import de.jplag.cli.test.CliTest;
import de.jplag.exceptions.ExitException;

public class ModeTest extends CliTest {
@Test
void testViewWithPositionalFile() throws IOException, ExitException {
CliInputHandler inputHandler = this
.runCli(args -> args.with(CliArgument.MODE, "view").with(CliArgument.SUBMISSION_DIRECTORIES, new String[] {"result.zip"}))
.inputHandler();
assertEquals(new File("result.zip"), inputHandler.getFileForViewMode());
}

@Test
void testViewWithOldFile() throws IOException, ExitException {
CliInputHandler inputHandler = this
.runCli(args -> args.with(CliArgument.MODE, "view").with(CliArgument.OLD_SUBMISSION_DIRECTORIES, new String[] {"result.zip"}))
.inputHandler();
assertEquals(new File("result.zip"), inputHandler.getFileForViewMode());
}

@Test
void testViewWithNewFile() throws IOException, ExitException {
CliInputHandler inputHandler = this
.runCli(args -> args.with(CliArgument.MODE, "view").with(CliArgument.NEW_SUBMISSION_DIRECTORIES, new String[] {"result.zip"}))
.inputHandler();
assertEquals(new File("result.zip"), inputHandler.getFileForViewMode());
}

@Test
void testViewWithResultFile() throws IOException, ExitException {
CliInputHandler inputHandler = this.runCli(args -> args.with(CliArgument.MODE, "view").with(CliArgument.RESULT_FILE, "result.zip"))
.inputHandler();
assertEquals(new File("result.zip"), inputHandler.getFileForViewMode());
}

@Test
void testViewWithMultipleFiles() throws IOException, ExitException {
assertThrowsExactly(CliException.class, () -> {
this.runCli(args -> args.with(CliArgument.MODE, "view").with(CliArgument.RESULT_FILE, "result.zip")
.with(CliArgument.NEW_SUBMISSION_DIRECTORIES, new String[] {"test.zip"})).inputHandler();
});
}

@Override
public void addDefaultParameters() {
}
}
2 changes: 2 additions & 0 deletions cli/src/test/java/de/jplag/cli/test/CliArgument.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,6 @@ public record CliArgument<T>(String name, boolean isPositional) {

public static CliArgument<String> SUBDIRECTORY = new CliArgument<>("subdirectory", false);
public static CliArgument<String> EXCLUDE_FILES = new CliArgument<>("x", false);

public static CliArgument<String> MODE = new CliArgument<>("mode", false);
}
3 changes: 2 additions & 1 deletion cli/src/test/java/de/jplag/cli/test/CliResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

import org.slf4j.event.Level;

import de.jplag.cli.picocli.CliInputHandler;
import de.jplag.options.JPlagOptions;

public record CliResult(JPlagOptions jPlagOptions, String targetPath, Level logLevel) {
public record CliResult(JPlagOptions jPlagOptions, String targetPath, Level logLevel, CliInputHandler inputHandler) {
}
2 changes: 1 addition & 1 deletion cli/src/test/java/de/jplag/cli/test/CliTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ protected CliResult runCli(Consumer<CliArgumentBuilder> additionalOptionsBuilder

String targetPath = (String) getWritableFileMethod.invoke(cli);

return new CliResult(optionsBuilder.buildOptions(), targetPath, CollectedLogger.getLogLevel());
return new CliResult(optionsBuilder.buildOptions(), targetPath, CollectedLogger.getLogLevel(), inputHandler);
} catch (IllegalAccessException | InvocationTargetException e) {
Assumptions.abort("Could not access private field in CLI for test.");
return null; // will not be executed
Expand Down
Loading