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

feat: Wrap web-ext usage output at terminal width #2329

Merged
merged 1 commit into from
Nov 23, 2021

Conversation

joelpurra
Copy link
Contributor

@joelpurra joelpurra commented Oct 20, 2021

  • Detects the terminal width (in characters) using yargs.terminalWidth().
  • Sets the generated yargs usage to wrap at the terminal width instead of at the default (maximum) 80 characters.
  • Should improve web-ext readability by utilizing the full terminal window, when it is greater than 80 characters wide.
  • In particular, --help output is improved.
  • Testing would require a TTY emulator. Tests were added and removed due to build system tooling issues.

See

Before: (maximum) 80 characters wide
Usage: web-ext [options] command

Option values can also be set by declaring an environment variable prefixed
with $WEB_EXT_. For example: $WEB_EXT_SOURCE_DIR=/path is the same as
--source-dir=/path.

To view specific help for any given command, add the command name.
Example: web-ext --help run.


Commands:
  web-ext build  Create an extension package from source
  web-ext sign   Sign the extension so it can be installed in Firefox
  web-ext run    Run the extension
  web-ext lint   Validate the extension source
  web-ext docs   Open the web-ext documentation in a browser

Options:
      --version           Show version number                          [boolean]
  -s, --source-dir        Web extension source directory.
                                [string] [required] [default: "/home/joelpurra"]
  -a, --artifacts-dir     Directory where artifacts will be saved.
              [string] [required] [default: "/home/joelpurra/web-ext-artifacts"]
  -v, --verbose           Show verbose output                          [boolean]
  -i, --ignore-files      A list of glob patterns to define which files should
                          be ignored. (Example: --ignore-files=path/to/first.js
                          path/to/second.js "**/*.log")                  [array]
      --no-input          Disable all features that require standard input
                                                                       [boolean]
  -c, --config            Path to a CommonJS config file to set option defaults
                                                                        [string]
      --config-discovery  Discover config files in home directory and working
                          directory. Disable with --no-config-discovery.
                                                       [boolean] [default: true]
  -h, --help              Show help                                    [boolean]

You must specify a command
After: 120 characters wide (example, autodetected)
Usage: web-ext [options] command

Option values can also be set by declaring an environment variable prefixed
with $WEB_EXT_. For example: $WEB_EXT_SOURCE_DIR=/path is the same as
--source-dir=/path.

To view specific help for any given command, add the command name.
Example: web-ext --help run.


Commands:
  web-ext build  Create an extension package from source
  web-ext sign   Sign the extension so it can be installed in Firefox
  web-ext run    Run the extension
  web-ext lint   Validate the extension source
  web-ext docs   Open the web-ext documentation in a browser

Options:
      --version           Show version number                                                                  [boolean]
  -s, --source-dir        Web extension source directory.               [string] [required] [default: "/home/joelpurra"]
  -a, --artifacts-dir     Directory where artifacts will be saved.
                                                      [string] [required] [default: "/home/joelpurra/web-ext-artifacts"]
  -v, --verbose           Show verbose output                                                                  [boolean]
  -i, --ignore-files      A list of glob patterns to define which files should be ignored. (Example:
                          --ignore-files=path/to/first.js path/to/second.js "**/*.log")                          [array]
      --no-input          Disable all features that require standard input                                     [boolean]
  -c, --config            Path to a CommonJS config file to set option defaults                                 [string]
      --config-discovery  Discover config files in home directory and working directory. Disable with
                          --no-config-discovery.                                               [boolean] [default: true]
  -h, --help              Show help                                                                            [boolean]

You must specify a command

@joelpurra joelpurra force-pushed the detect-terminal-width branch 2 times, most recently from d5d5230 to 5c352ca Compare October 20, 2021 11:07
@joelpurra joelpurra changed the title Wrap web-ext usage output at terminal width feat: Wrap web-ext usage output at terminal width Oct 20, 2021
@codecov
Copy link

codecov bot commented Oct 20, 2021

Codecov Report

Merging #2329 (5e85d22) into master (19fdd3a) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 5e85d22 differs from pull request most recent head 0c5e0e5. Consider uploading reports for the commit 0c5e0e5 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2329   +/-   ##
=======================================
  Coverage   99.88%   99.88%           
=======================================
  Files          32       32           
  Lines        1699     1700    +1     
=======================================
+ Hits         1697     1698    +1     
  Misses          2        2           
Impacted Files Coverage Δ
src/program.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 19fdd3a...0c5e0e5. Read the comment docs.

@Rob--W
Copy link
Member

Rob--W commented Oct 21, 2021

Could you add a unit test, so that this feature works as expected even if Yargs changes?

@joelpurra joelpurra force-pushed the detect-terminal-width branch from 5c352ca to 8932364 Compare November 17, 2021 14:18
@joelpurra
Copy link
Contributor Author

joelpurra commented Nov 17, 2021

@Rob--W: yargs writes directly to the terminal, which is not a TTY when running tests, so added pty. Apparently this requires Python minimum v3.6.0, which isn't available on the build server. I'm sure there's another TTY emulator which doesn't require additional dependencies; suggestions welcome. Don't really want to spend more time on this cosmetic change for (mainly) --help output.

npm ERR! gyp ERR! find Python checking if "python3" can be used
npm ERR! gyp ERR! find Python - executable path is "/usr/bin/python3"
npm ERR! gyp ERR! find Python - version is "3.5.3"
npm ERR! gyp ERR! find Python - version is 3.5.3 - should be >=3.6.0
npm ERR! gyp ERR! find Python - THIS VERSION OF PYTHON IS NOT SUPPORTED

Windows requires additional tooling.

@Rob--W
Copy link
Member

Rob--W commented Nov 17, 2021

Thanks for the test. Is it possible to just stub some (core) methods to get some minimal coverage? If not then I'm fine to land without tests.

@joelpurra
Copy link
Contributor Author

joelpurra commented Nov 17, 2021

@Rob--W: simple stubbing didn't help; suspect that yargs keeps internal references to console.log/process.stdout/etcetera. Removing test code.

joelpurra added a commit to joelpurra/web-ext that referenced this pull request Nov 17, 2021
- The test for `yargs` output width requires an emulated terminal.
- The pseudoterminal `node-pty` works, but requires Python >= v3.6.0.
- The build system has Python v3.5.3, so removing the tests.
- Can be reverted when the build system upgrades Python.

See

- mozilla#2329
- https://app.circleci.com/pipelines/github/mozilla/web-ext/1034/workflows/6522f202-759a-4d89-9f33-fdef054ad3ae/jobs/4181?invite=true#step-104-26
- https://github.com/microsoft/node-pty
Copy link
Member

@Rob--W Rob--W left a comment

Choose a reason for hiding this comment

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

Thanks.

joelpurra added a commit to joelpurra/web-ext that referenced this pull request Nov 17, 2021
- Detects the terminal width (in characters) using `yargs.terminalWidth()`.
- Sets the generated `yargs` usage to wrap at the terminal width instead
  of at the default (maximum) 80 characters.
- Should improve `web-ext` readability by utilizing the full terminal
  window, when it is greater than 80 characters wide.
- In particular, `--help` output is improved.
- Testing would require a TTY emulator. Tests were added and removed
  in mozilla#2329 due to build system tooling issues.

See

- https://yargs.js.org/docs/#api-reference-wrapcolumns
- mozilla#2329
@joelpurra joelpurra force-pushed the detect-terminal-width branch from 5e85d22 to e0bd9d5 Compare November 17, 2021 14:51
- Detects the terminal width (in characters) using `yargs.terminalWidth()`.
- Sets the generated `yargs` usage to wrap at the terminal width instead
  of at the default (maximum) 80 characters.
- Should improve `web-ext` readability by utilizing the full terminal
  window, when it is greater than 80 characters wide.
- In particular, `--help` output is improved.
- Testing would require a TTY emulator. Tests were added and removed
  in mozilla#2329 due to build system tooling issues.

See

- https://yargs.js.org/docs/#api-reference-wrapcolumns
- mozilla#2329
@joelpurra joelpurra force-pushed the detect-terminal-width branch from e0bd9d5 to 0c5e0e5 Compare November 17, 2021 15:00
@rpl rpl merged commit 858fe0d into mozilla:master Nov 23, 2021
@joelpurra joelpurra deleted the detect-terminal-width branch December 16, 2021 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants