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] Use newlines in list when stdout is a tty or when passed -1 #54

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

coreyja
Copy link
Contributor

@coreyja coreyja commented Nov 1, 2021

Hello again @brianp 👋

Been using muxed constantly for the past few years and continue to love it! 💜

I was looking to revamp some of my tmux stuff recently and was getting annoyed with my sed hacks to get newline delimited results out of list 😆

So here is proper support for tty detection, as well as an option to force one item per line (following lss -1 options)

I was going to look into adding some test cases for that, but realized the first road block is that we don't have a great way to capture stdout in the tests, which makes it hard to write good tests for this list sub-command.

I've used assert_cmd for this in the past, but not sure if there are better options or ways of doing this. Another option I found is maybe extracting out the writing to stdout. Maybe by passing in a writer that can write to stdout for the binary and a buffer for tests? (https://users.rust-lang.org/t/how-to-test-output-to-stdout/4877/3)

My Example with assert_cmd: https://github.com/coreyja/devicon-lookup/blob/master/tests/strip_ansi.rs (Uses a 1.x version it looks like, just FYI)
Crate Docs: https://docs.rs/assert_cmd/2.0.2/assert_cmd/

Do you have thoughts on how you would like to do testing?

Fixes #53

@brianp
Copy link
Owner

brianp commented Nov 2, 2021

🙌🏻 Cheers @coreyja and good to hear from you!

I just recently had a my first child which has put off my work on the other feature requests.

I'll see if I can't give this PR a run through this week/weekend. It looks good to me though so expect a merge soon. I'll also take a closer look at the stdout/ assert_cmd options. There's at least one other place I skipped testing because of the same issue. Thanks for presenting some options.

@coreyja
Copy link
Contributor Author

coreyja commented Nov 3, 2021

I just recently had a my first child

Congrats!!

which has put off my work on the other feature requests.

You weren't, but never need to apologize! This is already an amazing tool, so thank you for everything you've already done!

I'll see if I can't give this PR a run through this week/weekend. It looks good to me though so expect a merge soon.

Awesome, thanks!

good to hear from you!

Good to hear from you too, I hope all is well!

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.

[Feature Request]list when piped should be New Line deliminated
2 participants