@picnixz sorry for choosing you, I just haven't found anyone else from the core team who is active yet (I looked at who is involved in issue and experts table). could you review?

You can ping me for anything that is remotely related to security and cryptography

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phraseI have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@picnixz I chosemask because it was used in the issue examples and also it seems that*char assumes one character while our argument can take arbitrary length. But it's not very important for me.
As for using keyword-only argument, I don't quite understand why that is.

As for using keyword-only argument, I don't quite understand why that is.

Because if you were to use it, you would writegetpass(mask="*"), likely not changing the prompt nor the stream value. And keyword-only arguments is good for future compatibility (e.g., if we want to add more options, we are not held back by the order of the arguments)

I chose mask because it was used in the issue examples and also it seems that *char

Usually, you expect 1 character indeed. I don't think we should write something else than 1 char (or maybe we do but it's because of colors?) I would not expectwords to be put as placeholders. Maybe colorized characters, which in this case are allowed to be strings of length > 1, but not real words.

Also, what we are echoing is really the corresponding passwordcharacter. Foreach character, we use that placeholder.mask is really not a good idea because it could hint that we would (bitwise) mask the password (namely perform some& operation). But this is something we can think of at the end.

I have made the requested changes; please review again

Thanks for making the requested changes!

@picnixz: please review the changes made to this pull request.

@picnixz I just thought maybe we also should to add, *, echochar=None togetpass.fallback_getpass to have a unified interface

Thanks for making the requested changes!

@picnixz: please review the changes made to this pull request.

@picnixz, I added test cases, what do you think about them?

Can we have tests where we exercise control characters? this is quite important because we want to get the correct feedback

I added test case with control chars\t and\b. butmock_output.getvalue() have***\x08 \x08 in the end of string without removing* unlike the real console where* is removed by itself and\x08 is not displayed

Can we have tests where we exercise control characters? this is quite important because we want to get the correct feedback

I added test case with control chars\t and\b. butmock_output.getvalue() have***\x08 \x08 in the end of string without removing* unlike the real console where* is removed by itself and\x08 is not displayed

cc@picnixz

@picnixz It seems that all the tests that you requested already exist, but maybe it will be possible to add more since there is already an issue for covering the module with tests

I would like to have time to accept before feature freeze :-)

I am travelling (I'm back after the beta release) so I won't merge anything on mobile. I'll defer this to@gpshead, sorry but it passed under my radar :( (in the future, a request for review is more likely to be noticed)

Thank you everyone!