Fix #3477: Add custom classpath to dotr script #3493
Conversation
dist/bin/dotr
Outdated
| shift | ||
| ;; | ||
| -classpath) | ||
| CLASS_PATH+=":$2" |
There was a problem hiding this comment.
We should probably add the user-defined classpaths to the left of the classpath to give them the priority.
dist/bin/dotr
Outdated
| shift | ||
| ;; | ||
| -d) | ||
| DEBUG="$DEBUG_STR" |
There was a problem hiding this comment.
Is this ever reached with the *) before it?
dist/bin/dotr
Outdated
| echo "Starting dotty REPL..." | ||
| eval "$PROG_HOME/bin/dotc -repl" | ||
| elif [[ ${first_arg:0:1} == "-" ]]; then | ||
| eval "$PROG_HOME/bin/dotc -repl $@" |
There was a problem hiding this comment.
We should still be able to call the repl with a custom classpath and custom args. You can assume that the -repl is set in that case and hence run_repl should be true.
There was a problem hiding this comment.
Ok,
what about this behavior:
- If no arguments, start repl
- If additional arguments and the
-replflag, run repl with these arguments - Otherwise run it
I pushed this.
rsoeldner
changed the title
|
You could also enable the two tests in https://github.com/lampepfl/dotty/blob/master/project/scripts/sbtBootstrappedTests#L29. That way we can avoid some regressions |
|
Otherwise the script code LGTM |
dist/bin/dotr
Outdated
|
|
||
| if [ $run_repl == true ]; then | ||
| echo "Starting dotty REPL" | ||
| echo "$PROG_HOME/bin/dotc -classpath $CLASS_PATH -repl ${residual_args[@]}" |
There was a problem hiding this comment.
Here the classpath should only contain classpath_args if it is not empty.
What does it mean exactly classpath_args[@]? I'm not familiar with the [@] part.
There was a problem hiding this comment.
@nicolasstucki Afaik, the [*] or [@] subscripts differ only within double quotes, with [*] each element inside the array is seperated by IFS
|
@nicolasstucki If everything is ok, I would rebase to a single commit. |
rsoeldner
force-pushed
the
dotr-classpath
branch
from
dc4d818 to
732e2a8
Compare
|
I’d like to review this before we merge |
allanrenucci
force-pushed
the
dotr-classpath
branch
2 times, most recently
from
b78111f to
808dea5
Compare
allanrenucci
force-pushed
the
dotr-classpath
branch
from
808dea5 to
f49fbbe
Compare
allanrenucci
force-pushed
the
dotr-classpath
branch
from
f49fbbe to
a477cd5
Compare
|
@nicolasstucki can you review the last commit |
@nicolasstucki This addresses issue #3477