Skip to content

Conversation

@deboard
Copy link

@deboard deboard commented Jan 2, 2024

issue 2561, send all [bat error]s to stderr in default_error_handler match default.

@deboard
Copy link
Author

deboard commented Jan 3, 2024

So, here is my entry into the CHANGELOG.md:

The check is failing... Any ideas?

@keith-hall
Copy link
Collaborator

The CI logs show:

Grepping for PR info

I'm guessing it wants the PR # instead of the issue # in the changelog entry. So try with #2827 instead of #2561.

@deboard
Copy link
Author

deboard commented Jan 3, 2024 via email

@deboard
Copy link
Author

deboard commented Jan 3, 2024

This issue appears to be tied to if "attached_to_pager" is set to true in the Controller, stdout gets passed to default_error_handler (in error.rs) if so. A good fix may be to just always write "[bat error] {error}" to stderr in in the default section of the match in default_error_handler (error.rs@70).

I have tried this. It fixes the problem and doesn't doesn't mess with the pager functionality.

If you currently run bat like "bat --pager=never /tmp 2> /tmp/bat.txt", it handels redirection correctly.

exit 1
fi

exit 0
Copy link
Owner

Choose a reason for hiding this comment

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

Can we please turn this into a Rust-based integration test. See tests/integration_tests.rs and search for .stderr to see some examples.

Copy link
Author

Choose a reason for hiding this comment

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

Will do.

Copy link
Owner

Choose a reason for hiding this comment

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

This script is still here. Can it be removed?

Comment on lines +1207 to +1208
#[test]
fn send_all_bat_error_to_stderr() {}
Copy link
Owner

Choose a reason for hiding this comment

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

What is this?

Comment on lines +7 to +10
- Send all bat error messages to stderr, see #2827 (@deboard)

- Fix long file name wrapping in header, see #2835 (@FilipRazek)
-
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove the empty lines

@chicks-net
Copy link

This looks so close to being ready. Was there any particular reason this was abandoned?

@deboard
Copy link
Author

deboard commented Sep 26, 2024

I'll take a new look at it. I can't remember. Was I waiting for a review? Not sure. I'll get back at it.

@Enselic
Copy link
Collaborator

Enselic commented May 1, 2025

Closing as inactive for now.

@Enselic Enselic closed this May 1, 2025
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.

5 participants