collapse common logic into run-format formula #119
Conversation
|
I would not call myself a bash expert by any means but if what you said in the title works this is an amazing change and I would love to see it merged. |
format/private/format.sh
Outdated
| @@ -45,7 +51,7 @@ function ls-files { | |||
| 'SQL') patterns=('*.sql' '*.cql' '*.ddl' '*.inc' '*.mysql' '*.prc' '*.tab' '*.udf' '*.viw') ;; | |||
| 'Scala') patterns=('*.scala' '*.kojo' '*.sbt' '*.sc') ;; | |||
| 'Shell') patterns=('.bash_aliases' '.bash_functions' '.bash_history' '.bash_logout' '.bash_profile' '.bashrc' '.cshrc' '.flaskenv' '.kshrc' '.login' '.profile' '.zlogin' '.zlogout' '.zprofile' '.zshenv' '.zshrc' '9fs' 'PKGBUILD' 'bash_aliases' 'bash_logout' 'bash_profile' 'bashrc' 'cshrc' 'gradlew' 'kshrc' 'login' 'man' 'profile' 'zlogin' 'zlogout' 'zprofile' 'zshenv' 'zshrc' '*.sh' '*.bash' '*.bats' '*.cgi' '*.command' '*.fcgi' '*.ksh' '*.sh.in' '*.tmux' '*.tool' '*.trigger' '*.zsh' '*.zsh-theme') ;; | |||
| 'Starlark') patterns=('BUCK' 'BUILD' 'BUILD.bazel' 'MODULE.bazel' 'Tiltfile' 'WORKSPACE' 'WORKSPACE.bazel' '*.bzl' '*.star') ;; | |||
| 'Starlark') patterns=('BUCK' '**/BUCK' 'BUILD' '**/BUILD' 'BUILD.bazel' '**/BUILD.bazel' 'MODULE.bazel' 'Tiltfile' '**/Tiltfile' 'WORKSPACE' 'WORKSPACE.bazel' '*.bzl' '*.star') ;; | |||
There was a problem hiding this comment.
You will be able to get rid of the "**" and replace it with "*" once #124 lands for consistency.
There was a problem hiding this comment.
the ** isn't to prevent the accidental glob expansion by bash addressed in #124 (that problem is fixed on line 71 by wrapping the pattern array expansion in quotes), but to get git ls-files to recurse in searches. there does appear to be some inconsistencies in pattern matching in git- this is what i found to work correctly.
008c75e
to
5720029
Compare
|
updated to pass tests |
|
@alexeagle got a moment to give this a once-over, or at least let me know if there's interest? |
|
Yes! sorry for the radio silence, this is indeed an important improvement. How is the output handled? My biggest concern is that we shouldn't interleave outputs in a confusing way, rather each background job has to buffer and then we print the buffer after that job has completed. |
format/private/format.sh
Outdated
| ( echo "$files" | tr \\n \\0 | JAVA_RUNFILES="${RUNFILES_MANIFEST_FILE%_manifest}" xargs -0 "$bin" $args >&2 ; times ) | (read _ _ ; read tuser tsys; echo "Formatted ${lang} in ${tuser}" ) | ||
| ;; | ||
| Swift) | ||
| # for any linter that must be silenced |
format/private/format.sh
Outdated
| done | ||
| fi | ||
| ( | ||
| trap "kill 0" SIGINT |
There was a problem hiding this comment.
could we break this up into two PRs please?
Let's land the refactoring to introduce the run-lints function first, but not run them in parallel yet. That way you don't get stuck with any messy rebases while we figure out how to buffer the output properly.
There was a problem hiding this comment.
have removed the parallelization for the moment. kind of nice how little difference there is.
|
got any suggestions as to how you'd like it to look? |
|
I think we can't print anything like "Formatting x with y..." because those will all appear at the top as the subshells are spawned in the background. And when a tool completes I think we want to print the full buffer it produced. |
|
Right, so the simple thing in Bash is to redirect the stdout/stderr of the individual tools to a file. Then when the tool is finished we can It's tempting to rewrite it in a language where there's a nice terminal experience available in some library, like superconsole. It would be a big effort though. |
|
sorry, merge conflicts now. would be nice to get the refactor in first :) |
|
busy week, sorry, will look later today
…On Thu, Feb 8, 2024 at 2:55 PM Alex Eagle ***@***.***> wrote:
sorry, merge conflicts now. would be nice to get the refactor in first :)
—
Reply to this email directly, view it on GitHub
<#119 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAAE7QOPH6M7XSFLEJQ45DYSUUT7AVCNFSM6AAAAABCLQ3TDOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMZUHAZTQOBYGQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
5720029
to
7cf1cfb
Compare
abafa1f
to
d00b0d8
Compare
step on the road to parallelizing the formatters
d00b0d8
to
84608ef
Compare
took a crack at parallelizing the formatter script, i think this works OK.
Type of change
Test plan