improved readability of help messages

[michelou]

This PR aims to improve the readability of help messages.

Concretely it addresses 2 weaknesses of the current behavior:

1. In the presence of a very long option name, e.g. -scalajs-genStaticForwardersForNonTopLevelObjects, all descriptions of the selected option subset are currently displayed with a large empty space on their left.

   NB. Alternative (and cheap) solution: define a maximum length for option names.

2. If the text length of the description is greater than the terminal width minus the largest option width (which not exceeds 30 chars), the rear part of the text flows to the next line, starting at column 0.

   NB. javac mixes two ways to display descriptions in help messages (see e.g. javac -X): descriptions are either programmatically split into several lines or their rear part simply flows to the next line, starting at column 0.

Example for case 1:

$ c:\opt\scala-3.0.0-M1\bin\scalac -help
Usage: scalac <options> <source files>
where possible standard options include:
-P                                                Pass an option to a plugin, e.g. -P:<plugin>:<opt>
-X                                                Print a synopsis of advanced options.
-Y                                                Print a synopsis of private options.
-bootclasspath                                    Override location of bootstrap class files.
[...]
-scalajs-genStaticForwardersForNonTopLevelObjects Generate static forwarders even for non-top-level objects (Scala.js only)
-semanticdb-target                                Specify an alternative output directory for SemanticDB files.
[...]
-verbose                                          Output messages about what the compiler is doing.
-version                                          Print product version and exit.

The modified behavior for case 1 looks as follows:

Usage: scalac <options> <source files>
where possible standard options include:
-P                 Pass an option to a plugin, e.g. -P:<plugin>:<opt>
-X                 Print a synopsis of advanced options.
-Y                 Print a synopsis of private options.
-bootclasspath     Override location of bootstrap class files.
[...]
-scalajs           Compile in Scala.js mode (requires scalajs-library.jar on the classpath).
-scalajs-genStaticForwardersForNonTopLevelObjects
                   Generate static forwarders even for non-top-level objects (Scala.js only)
-semanticdb-target Specify an alternative output directory for SemanticDB files.
[...]
-verbose           Output messages about what the compiler is doing.
-version           Print product version and exit.

Example for case 2 with a terminal width of 100 chars:

$ c:\opt\scala-3.0.0-M1\bin\scalac -Y
Usage: scalac <options> <source files>

    |-- Notes on option parsing --
    [...]

Possible private options include:
-Ycheck                  Check the tree at the end of
-Ycheck-all-patmat       Check exhaustivity and redundancy of all pattern matching (used for testing
 the algorithm).
-Ycheck-init             Check initialization of objects
-Ycheck-mods             Check that symbols and their defining trees have modifiers in sync.
-Ycheck-reentrant        Check that compiled program does not contain vars that can be accessed from
 a global root.
-Ycook-comments          Cook the comments (type check `@usecase`, etc.)
-Ydebug                  Increase the quantity of debugging output.
[...]
-Ytrace-context-creation Store stack trace of context creations.

The modified behavior for case 2 looks as follows:
NB. Resize your terminal and execute the command again for see the updated layout of the help message.

$ c:\opt\scala-3.0.0-M1\bin\scalac -Y
Usage: scalac <options> <source files>

    |-- Notes on option parsing --
    [...]

Possible private options include:
-Ycheck                  Check the tree at the end of
-Ycheck-all-patmat       Check exhaustivity and redundancy of all pattern matching (used for
                         testing the algorithm).
-Ycheck-init             Check initialization of objects
-Ycheck-mods             Check that symbols and their defining trees have modifiers in sync.
-Ycheck-reentrant        Check that compiled program does not contain vars that can be accessed
                         from a global root.
[...]
-Ytrace-context-creation Store stack trace of context creations.

Implementation notes:

1. The PR adds the external dependency org.jline to object dotty.tools.dotc.CompilerCommand (needed to get the terminal width).

2. The PR keeps the descriptions untouched if the help message is redirected to a file, e.g. scalac -help > help.txt.

3. The PR does not handle the two fields default and legalChoices (class Setting) and keeps the notes on option parsing (-X/-Y) untouched.

[dottybot]

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Have an awesome day! ☀️

[sjrd]

Not sure what I can contribute here ^^ I mean the output looks good.

I guess the compiler was already depending on JLine for the REPL anyway, so it is probably not a big deal.

[michelou]

@sjrd Thanks for your feedback. I added you as a reviewer by virtue of option -scalajs-genStaticForwardersForNonTopLevelObjects 😉

[smarter]

While the improved output is nice, I'm concerned about this patch because it seems to significantly increase the startup time of scalac -help: on my machine on master it takes between 0.5 and 0.6 seconds to run (measures come from the real column of /usr/bin/time -p), whereas with this PR it takes between 0.6 and 0.7 seconds. For comparison scalac 2.13 takes about 0.4 seconds to display its help (scalac 2.11 is even faster: 0.2 seconds, but the 2.12+ trait encoding slowed things down). So we've already regressed compared to Scala 2 and I'd rather not regress even more.

[smarter]

To be clear: it's highly likely the only way to avoid the startup time is probably to not use JLine at all. Instead, I suggest relying on the existing -pagewidth setting to determine the maximum width.

[michelou]

@smarter I agree.

[smarter]

Checking these environment variables seems like a good idea, but instead of doing it here, we could use them to set a default value for the pagewidth setting instead of 80, that way we'll use them in other situations where we're printing things: https://github.com/lampepfl/dotty/blob/6b9796c38c37957c6a6dba387c4a820b06976356/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala#L44

[smarter]

Otherwise LGTM!

[smarter]

On Linux at least, this will not be set inside a shell script by default, the best portable way I've found to deal with that is to set it with:

if command -v tput >/dev/null 2>&1; then
    export COLUMNS="$(tput -Tdumb cols)"
fi

in https://github.com/lampepfl/dotty/blob/master/dist/bin/common (I checked and that doesn't impact performance).

[michelou]

@smarter I know that tput usage but I was hesitant to add it to the Scala 3 launch script. I'm fine if it's ok for you.

I suggest the following change:

if [ -z ${COLUMNS+x} ] && command -v tput >/dev/null 2>&1; then
    export COLUMNS="$(tput -Tdumb cols)"
fi

since it looks like running tput only once per session is enough to activate the automatic update of variable COLUMNS.

Can you please confirm the above behavior in your Unix environment ?!
For instance (on your command line, after executing the above command):

1. $ set COLUMNS=
2. $ echo $COLUMNS (should print the actual #columms in your terminal)
3. Resize your terminal window.
4. $ echo $COLUMNS (should print the updated #columms in your terminal)

[michelou]

Ok, I will add your suggested code for now.

[smarter]

Sorry for the delay, looks like I forgot to answer here!

since it looks like running tput only once per session is enough to activate the automatic update of variable COLUMNS.

I think that the shell itself is responsible for updating $COLUMNS, if I'm in zsh or bash, $COLUMNS get updated automatically even if I reset it, but in dash for example it never gets updated.

[liufengyun]

@michelou Do you need a rebase against the master?

[michelou]

@michelou Do you need a rebase against the master?

I'll have a look (no activity since november 2020...).

[griggt]

@michelou Would you like me to try pushing a commit to bring the community build submodules up-to-date on this PR, to fix the CI failures?

[michelou]

@griggt My local branch scala3-help seems to be broken ( I'm working on MS Windows). It's the 3rd time (aka PR) I fall into such troubles (in the previous cases I ended up deleting the branch and recreating a new one with my saved changes).

Your help is welcome (one job is still running). I could not find what I'm doing wrong with the submodules of the community build (I never touch them).

[griggt]

The commit I just pushed should take care of things in your fork on GitHub (and this PR), hopefully you will be able to sync successfully to your local branch as well.

As to how the submodules got out of sync to begin with, I have no idea. But to echo a sentiment shared by others: "submodules are the worst".

[michelou]

@smarter Please review and merge if everything is ok for you !
PS. Thanks to @griggt for solving my (borimg) issues with submodules.

[smarter]

LGTM, thanks for working on this!

By the way, you can use "git rebase" or "git pull --rebase" to avoid introducing merge commits when updating your PR, I'll take care of squashing everything to one commit here.

[michelou]

LGTM, thanks for working on this!

By the way, you can use "git rebase" or "git pull --rebase" to avoid introducing merge commits when updating your PR, I'll take care of squashing everything to one commit here.

@smarter I've been using those commands with various options during the last days.. but I was not able to reach a stable (up-to-date) state. As I said previously @griggt solved my issue FINALLY !

[smarter]

The problem with submodules is that they don't get updated when you rebase or merge changes, you need to do git submodule update too, if you don't and then do a git commit -a, you'll accidentally commit the old versions of the submodules. You can sidestep the problem by disabling submodules when you're not using them: for i in community-build/community-projects/*; do git submodule deinit $i; done