From the CLI, we have scoped and unscoped argument...
# development
w
From the CLI, we have scoped and unscoped arguments for each of the Pants goals... Is there a reason why we have both? The top-level help shows the scoped one, but the examples use the unscoped versions - so I don't think I understand why we have both.
Copy code
% ./pants help fmt  

`fmt` goal options
------------------

Autoformat source code.
 
Activated by pants.core
Config section: [fmt]
 
  --fmt-only="['<str>', '<str>', ...]"
  PANTS_FMT_ONLY
  only
      default: []
      current value: []
      Only run these formatters and skip all others.
      
      The formatter names are outputted at the final summary of running this goal, e.g. `isort` and `shfmt`. You can also run `fmt --only=fake` to get a list of all activated formatters.
      
      You can repeat this option, e.g. `fmt --only=isort --only=shfmt` or `fmt --only=['isort', 'shfmt']`.
b
In general I like to think in scoped options. But occasionally I'm writing wrapper scripts and it's nice that I can compose them willy-nilly and not worry so much about positioning. No comment on the history though, just my observation
w
So, it's more about positioning than anything else?
b
In my worldview, I'd say so. Others likely have other worldviews
w
Fair enough, just thought they would have been placed under the global options in that case - but I would also understand how hard that would get to read
h
By unscoped you mean the shorthand of
./pants goal --foo ::
instead of
./pants --goal-foo goal ::
?
w
Yes, I think in the
help-all
it's marked as
unscoped/scoped_cmd_line_args
This is for the CLI tab completion PR I'm pushing this evening. I was thinking to maybe show the scoped args in the global options, but that gets a bit messy, while the unscoped args are immediately after the goal
b
I think for developers typing commands, thinking scoped args is best. The only oddity is specs-options like the
changed
subsystem
c
+1 for scoped, as I think that will be easier and more robust to implement tab completion for where you don't have to consider which goal it would apply to as unscoped otherwise.
w
I could also show both, but that definitely starts to look whacky. FWIW, I've always used scoped, because I didn't even realize unscoped was a thing
p
I knew about unscoped, but I keep forgetting. So every time I look in the docs or help it shows the scoped version so I use that.
h
Yeah, probably scoped is best to start with because scoped flags work everywhere on the cmd line. Unscoped shorthand can be a power user thing.
w
I'll sanity test a few options - once the infra is there, it'll be pretty easy to change
WIP PR: https://github.com/pantsbuild/pants/pull/16200 It may be a macos thing, or maybe a Bash 3 thing - but I can't do some of the neat var substitution that I wanted to, which would make the generated completion file about half the size, but 🤷
f
If it helps, the Pants v1 version of shell autocompletion would parse out the goal, and try to autocomplete the goal-specific options.
so there is a prior example of some “fun” bash code to accomplish that
w
parse out the goal, and try to autocomplete the goal-specific options
That's what this version can do. My test that seems to work well is goal options, and putting scoped ones in the global namespace (working on this part) - best of both worlds, more or less. Also gets a bit weird, because after each goal, I think you can validly put global options in there, but they're not auto-completed
b
Yeah I think making the assumption that global args come after pants but before a goal is ok IMO
w
I'd hope so, pulls in a half-reasonable auto-complete convention, and nothing breaks, so. 🤷
Oof
Copy code
% pants --
zsh: do you wish to see all 220 possibilities (37 lines)?
💪 1
Didn't realize pants was so baggy
😂 1
🙂
b
A lot of subsystems yo
w
Nope - subsystems aren't included, other than "changed" - just a ton of configurability
b
220 seems high then. We don't have 220 global options.
w
Global options + scoped goal options, so we don't lose those
b
Shouldn't it be the other way?
Should be Global + Subsystem, but not scoped goal options
w
Copy code
% pants --
--auth-acquire-description                     --filter-address-regex                         --no-dependencies-transitive                   --no-update-build-files-check                  --publish-output                               --spec-files
--auth-acquire-headless                        --filter-granularity                           --no-dynamic-ui                                --no-update-build-files-fix-safe-deprecations  --py-constraints-output-file                   --stats-record-option-scopes
--auth-acquire-local-port                      --filter-output-file                           --no-experimental-bsp-server                   --no-update-build-files-fmt                    --py-constraints-summary                       --streaming-workunits-complete-async
--auth-acquire-output                          --filter-sep                                   --no-filedeps-absolute                         --no-verify-config                             --pythonpath                                   --streaming-workunits-level
--auth-acquire-test-page                       --filter-tag-regex                             --no-filedeps-globs                            --no-watch-filesystem                          --remote-auth-plugin                           --streaming-workunits-report-interval
--backend-packages                             --filter-target-type                           --no-filedeps-transitive                       --pants-bin-name                               --remote-ca-certs-path                         --subproject-roots
--build-file-prelude-globs                     --fmt-batch-size                               --no-lint-skip-formatters                      --pants-config-files                           --remote-cache-eager-fetch                     --tag
--build-ignore                                 --fmt-only                                     --no-list-documented                           --pants-distdir                                --remote-cache-read                            --tailor-alias-mapping
--build-patterns                               --generate-lockfiles-custom-command            --no-local-cache                               --pants-ignore                                 --remote-cache-read-timeout-millis             --tailor-build-file-header
--ca-certs-path                                --generate-lockfiles-resolve                   --no-log-show-rust-3rdparty                    --pants-ignore-use-gitignore                   --remote-cache-rpc-concurrency                 --tailor-build-file-indent
--changed-dependees                            --ignore-warnings                              --no-loop                                      --pants-physical-workdir-base                  --remote-cache-warnings                        --tailor-build-file-name
--changed-diffspec                             --level                                        --no-pants-ignore-use-gitignore                --pants-subprocessdir                          --remote-cache-write                           --tailor-check
--changed-since                                --lint-batch-size                              --no-pantsd                                    --pants-version                                --remote-execution                             --tailor-ignore-adding-targets
--check-only                                   --lint-only                                    --no-pantsrc                                   --pants-workdir                                --remote-execution-address                     --tailor-ignore-paths
--colors                                       --lint-skip-formatters                         --no-peek-exclude-defaults                     --pantsd                                       --remote-execution-extra-platform-properties   --test-debug
--concurrent                                   --list-documented                              --no-plugins-force-resolve                     --pantsd-invalidation-globs                    --remote-execution-headers                     --test-debug-adapter
--dependees-closed                             --list-output-file                             --no-print-stacktrace                          --pantsd-max-memory-usage                      --remote-execution-overall-deadline-secs       --test-extra-env-vars
--dependees-output-file                        --list-sep                                     --no-process-cleanup                           --pantsd-pailgun-port                          --remote-execution-rpc-concurrency             --test-force
--dependees-sep                                --local-cache                                  --no-process-execution-local-enable-nailgun    --pantsd-timeout-when-multiple-invocations     --remote-instance-name                         --test-open-coverage
--dependees-transitive                         --local-execution-root-dir                     --no-py-constraints-summary                    --pantsrc                                      --remote-oauth-bearer-token-path               --test-output
--dependencies-closed                          --local-store-dir                              --no-remote-cache-eager-fetch                  --pantsrc-files                                --remote-store-address                         --test-report
--dependencies-output-file                     --local-store-directories-max-size-bytes       --no-remote-cache-read                         --paths-from                                   --remote-store-batch-api-size-limit            --test-report-dir
--dependencies-sep                             --local-store-files-max-size-bytes             --no-remote-cache-write                        --paths-output-file                            --remote-store-chunk-bytes                     --test-shard
--dependencies-transitive                      --local-store-processes-max-size-bytes         --no-remote-execution                          --paths-to                                     --remote-store-chunk-upload-timeout-seconds    --test-timeout-default
--dynamic-ui                                   --local-store-shard-count                      --no-repl-restartable                          --peek-exclude-defaults                        --remote-store-headers                         --test-timeout-maximum
--dynamic-ui-renderer                          --log-levels-by-target                         --no-run-cleanup                               --peek-output-file                             --remote-store-rpc-concurrency                 --test-timeouts
--engine-visualize-to                          --log-show-rust-3rdparty                       --no-run-debug-adapter                         --plugins                                      --remote-store-rpc-retries                     --test-use-coverage
--experimental-bsp-groups-config-files         --logdir                                       --no-show-log-target                           --plugins-force-resolve                        --repl-restartable                             --unmatched-build-file-globs
--experimental-bsp-runner-env-vars             --loop                                         --no-streaming-workunits-complete-async        --print-stacktrace                             --repl-shell                                   --unmatched-cli-globs
--experimental-bsp-server                      --loop-max                                     --no-tailor-check                              --process-cleanup                              --roots-output-file                            --update-build-files-check
--experimental-explorer-address                --named-caches-dir                             --no-test-debug                                --process-execution-cache-namespace            --roots-sep                                    --update-build-files-fix-safe-deprecations
--experimental-explorer-port                   --no-auth-acquire-headless                     --no-test-debug-adapter                        --process-execution-graceful-shutdown-timeout  --rule-threads-core                            --update-build-files-fmt
--filedeps-absolute                            --no-colors                                    --no-test-force                                --process-execution-local-enable-nailgun       --rule-threads-max                             --update-build-files-formatter
--filedeps-globs                               --no-concurrent                                --no-test-open-coverage                        --process-execution-local-parallelism          --run-args                                     --verify-config
--filedeps-output-file                         --no-dependees-closed                          --no-test-report                               --process-execution-remote-parallelism         --run-cleanup                                  --watch-filesystem
--filedeps-sep                                 --no-dependees-transitive                      --no-test-timeouts                             --process-per-child-memory-usage               --run-debug-adapter                            
--filedeps-transitive                          --no-dependencies-closed                       --no-test-use-coverage                         --process-total-child-memory-usage             --show-log-target
b
Since those will likely be provided after the goal
Ahh also
--foo
and
--no-foo
yeah OK 😛
So really if you switch it to global + subsystem, this will likely exceed 220 😂
w
After the goal, we have unscoped options - since location matters, scoped options are basically location irrelevant. e.g.
./pants --changed-since=origin/main --lint-skip-formatters=... fmt --only=...
b
Right, that command doesn't make much sense 😉
w
Yeah, just auto-completing some randomness
In lieu of something like
./pants fmt --fmt-only
b
Right. I'm saying we likely shouldn't auto-complete scoped options.... anywhere. Only ever scoped options, and only after the relevant goal
Also, we should support subsystem options
w
I'm saying we likely shouldn't auto-complete scoped options.... anywhere. Only ever scoped options, and only after the relevant goal
Should one of these say unscoped? e.g. "scoped_cmd_line_args": ["--fmt-only"], "unscoped_cmd_line_args": ["--only"],
we should support subsystem options
Such as?
b
for 1: I'm saying we shouldn't ever suggest
--fmt-only
, just
fmt --only
for 2, IDK,
--docker-env-vars=...
or
--pylint-skip
or
--isort-args=[...]
w
There are some comments above that the preference appears to be only scoped because they work everywhere. In any case, I have no strong opinion on any of this. I'll upload what I have, and moving options around is pretty trivial
For anyone who wants to mess around, here are the WIP completions. Can source the file and try it out
@bitter-ability-32190 With subsystems included on the pants main repo 🙂
Copy code
% ./pants -
zsh: do you wish to see all 627 possibilities (157 lines)?
It's the worst case, I know - but man is that funny to see
Actually, even in one of my play repos, I'm still at 485 options, mostly from all the fmt/lint/check plugins in python
b
Yeah, that's more like it
I'm ready for 1k 😂
w

https://media.giphy.com/media/D3OdaKTGlpTBC/giphy.gif

😂 2