Skip to content

Add custom writer #36

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.

Sign up for GitHub

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

Jump to bottom
Merged
ynqa merged 5 commits into from
Feb 19, 2025
Merged

Add custom writer #36

ynqa merged 5 commits into from
Feb 19, 2025
+111 −30

Conversation

@cih9088
Copy link
Contributor

@cih9088 cih9088 commented Oct 26, 2024 •
edited
Loading

Rationale

Give users a bit of freedom to choose std::io::Write.
One specific use case is to pipe stdout and keep promkit interactive by rendering promkit to stderr.

Limitation

One caveat is that piping stdout does not work for now because cross term::cursor::position() uses stdout by default (crossterm-rs/crossterm#919).

Although it is not possible to pipe stdout due to the limitation of crossterm for the moment (hopefully fixed in near future), I believe this would be useful.

For custom use, I just added termion to get cursor position

@ynqa
Copy link
Owner

ynqa commented Dec 1, 2024 •
edited
Loading

@cih9088 First of all, thanks for your interests in this project and contributions 🎉

I believe I understand what you're trying to achieve. On the other hands, I would prefer not to attach anything to its arguments as much as possible (prompt() is similar to build method in the builder pattern).

Please follow the approach of adding fields to manage io::Write for each preset, along with methods to modify them, just like with other fields before that.

@cih9088 cih9088 force-pushed the feature/custom-writer branch from 4c87d0c to b9eaa95 Compare December 5, 2024 06:41
@cih9088
Copy link
Contributor Author

cih9088 commented Dec 5, 2024 •
edited
Loading

@ynqa

Changed to follow builder pattern.
Please feel free to change the naming of setter.

ynqa
ynqa approved these changes Feb 19, 2025
View reviewed changes
Copy link
Owner

@ynqa ynqa left a comment

Choose a reason for hiding this comment

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

@cih9088 Sorry for my review late🧎LGTM!!

@ynqa ynqa merged commit e034975 into ynqa:main Feb 19, 2025
@ynqa ynqa mentioned this pull request Feb 26, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ynqa ynqa ynqa approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@cih9088 @ynqa