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

age: Replace callback input read_line_initial_text with write_str and read_line #406

Merged
merged 2 commits into from Aug 7, 2023
Merged

age: Replace callback input read_line_initial_text with write_str and read_line #406

merged 2 commits into from Aug 7, 2023

Conversation

thibmeu
Copy link
Contributor

@thibmeu thibmeu commented Aug 7, 2023

read_line_initial_text fills the tty input with the initial text. This implies that the returned input is usually prefixed with the initial text, or that the initial text can be removed.
This differs from age behaviour, which is to not allow the initial text to be removed.

As a consequence, plugin leveraging callbacks for reading public input (confirm, request_public_string) exhibit a different behaviour depending on if they are called with age or rage.

This commit replaces read_line_initial_text with write_str and read_line. The initial text is still displayed to the user, but it does not affect the returned output.

…te_str` and `read_line`

`read_line_initial_text` fills the tty input with the initial text. This
implies that the returned input is usually prefixed with the initial
text, or that the initial text can be removed.
This differs from `age` behaviour, which is to not allow the initial
text to be removed.

As a consequence, plugin leveraging callbacks for reading public input
(confirm, request_public_string) exhibit a different behaviour depending
on if they are called with `age` or `rage`.

This commit replaces `read_line_initial_text` with `write_str` and
`read_line`. The initial text is still displayed to the user, but it
does not affect the returned output.
@str4d
Copy link
Owner

str4d commented Aug 7, 2023

This is not at all how I thought console::Term::read_line_initial_text worked; I thought the initial text was a description being displayed in front of the text input, not forming part of the response. Good catch, thanks!

@codecov
Copy link

codecov bot commented Aug 7, 2023

Codecov Report

Merging #406 (634e567) into main (ea42d81) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #406      +/-   ##
==========================================
- Coverage   40.88%   40.87%   -0.02%     
==========================================
  Files          34       34              
  Lines        3248     3249       +1     
==========================================
  Hits         1328     1328              
- Misses       1920     1921       +1     
Files Changed Coverage Δ
age/src/cli_common.rs 13.68% <0.00%> (-0.15%) ⬇️

... and 2 files with indirect coverage changes

@str4d str4d added this to the rage 0.10.0 milestone Aug 7, 2023
@str4d str4d changed the title age-plugin: Replace callback input read_line_initial_text with write_str and read_line age: Replace callback input read_line_initial_text with write_str and read_line Aug 7, 2023
Copy link
Owner

@str4d str4d left a comment

Choose a reason for hiding this comment

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

utACK, thanks!

@str4d str4d merged commit 488212d into str4d:main Aug 7, 2023
10 of 12 checks passed
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.

None yet

2 participants