Skip to content

Don't animate ActivityIndicator when outputting to a file#202

Merged
0xTim merged 3 commits intovapor:mainfrom
jflan-dd:jflan/quite-bars
Aug 20, 2024
Merged

Don't animate ActivityIndicator when outputting to a file#202
0xTim merged 3 commits intovapor:mainfrom
jflan-dd:jflan/quite-bars

Conversation

@jflan-dd
Copy link
Copy Markdown
Contributor

@jflan-dd jflan-dd commented Jul 15, 2024

These changes are now available in 4.15.0

Long running loading/progress indicators create a lot of noisy output when an executable is outputting to Xcode's console, being captured to a file, or a piped to another command. e.g. #141

This PR skips writing the animated output of an ActivityIndicator if the console will not handle it well.

Jul-15-2024 09-58-42

@jflan-dd jflan-dd requested review from 0xTim and gwynne as code owners July 15, 2024 14:59
@jflan-dd
Copy link
Copy Markdown
Contributor Author

@0xTim or @gwynne could you review this change when you get a chance?

@jflan-dd
Copy link
Copy Markdown
Contributor Author

jflan-dd commented Aug 5, 2024

@0xTim or @gwynne is there someone that could review this PR?

Copy link
Copy Markdown
Member

@0xTim 0xTim left a comment

Choose a reason for hiding this comment

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

Thanks! Sorry for the delay, looks good to me

@0xTim 0xTim added the semver-minor When merged, a new minor version release will be generated label Aug 20, 2024
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 54.26%. Comparing base (479a6af) to head (1c0ab67).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #202      +/-   ##
==========================================
- Coverage   56.32%   54.26%   -2.07%     
==========================================
  Files         101      103       +2     
  Lines        4495     4515      +20     
==========================================
- Hits         2532     2450      -82     
- Misses       1963     2065     +102     
Files Coverage Δ
...rces/ConsoleKitTerminal/Activity/ActivityBar.swift 62.50% <100.00%> (-12.50%) ⬇️
...onsoleKitTerminal/Activity/ActivityIndicator.swift 38.46% <100.00%> (-27.50%) ⬇️
Sources/ConsoleKitTerminal/Terminal/Console.swift 100.00% <100.00%> (ø)
Sources/ConsoleKitTerminal/Terminal/Terminal.swift 22.09% <100.00%> (-3.47%) ⬇️

... and 8 files with indirect coverage changes

@0xTim 0xTim merged commit 78c0dd7 into vapor:main Aug 20, 2024
@jflan-dd jflan-dd deleted the jflan/quite-bars branch August 20, 2024 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver-minor When merged, a new minor version release will be generated

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants