RevBank uses atomic file replacement by creating a new file and renaming
it over the old one. The newly created file is always in cwd, and
for the atomic rename() to work it must reside on the same filesystem as
the file it's replacing. Since File::Temp does the right thing and
creates files in /tmp by default, and /tmp is usually on a different
filesystem, these unit tests didn't actually work.
I don't know why they did work in the past. There doesn't seem to have
been any relevant change (or any at all, for that matter) to File::Temp,
which has had this behavior for ages. But I can't imagine that my /tmp
has only recently become a tmpfs mount either.
In any case, the issue is fixed by making File::Temp do the wrong thing,
which is to create its files in the cwd.
A sufficiently big number, i.e. longer than a long long, had interesting
effects. Perl would promote it to a float, and format it as -1 in
sprintf, which RevBank::Amount didn't handle correctly. In extreme cases
the number got rounded to Inf and would no longer round-trip.
As a result, numbers returned by RevBank::Amount are now Math::BigInt
and Math::BigFloat objects. Those should be transparent to all existing
code. It's amazing to see the unit tests pass.
I don't think there is any actual use case in RevBank for numbers this
large and I don't think anyone will have actually encountered the
aforementioned weird effects. Mostly, the input would be parsed with
parse_amount which refuses any number greater than 99900 anyway. Only
where parse_string was used directly, such large numbers could actually
have been used, but in stock RevBank that is only done when reading the
accounts file.
This change also introduces a new global function parse_any_amount that
is like parse_amount but doesn't complain about negative or large
numbers, to further improve the adduser plugin (see previous commit) in
insane edge cases. It differs from RevBank::Amount->parse_string in that
it does support addition and subtraction operators.
Old message was not as intended:
> Name for the new account: 123123123123
> That's way too much money. Enter 'abort' to abort.
Fixed:
> Name for the new account: 123123123123
Sorry, that's too numeric to be a user name. Enter 'abort' to abort.
Since the first versions of RevBank, negative and huge amounts are
handled centrally, and since v2 (2013) they've been implemented through
an exception that caused the pending transaction to be aborted. Since v3
(2019), RevBank has had a retry mechanism for rejected input to improve
the user experience, but it required a REJECT return message from a
plugin, not an exception. Now there's an exception class to trigger the
same semantics.
Commit 52749df5 added more information to error messages to aid
debugging, but most plugin follow-up questions are code references, not
method names, and they would result in an ugly CODE(0x...) in the error
message.
This change adds the fully qualified name of plugin methods. Not sure if
I like that, I might drop the RevBank::Plugin:: prefix at some point.
This somehow escaped change with the introduction of RevBank::Amount in
v3.2 in 2021, which only now became relevant due to the recent change in
v6.1.0 which turns invalid account balances into a feature.
This fixes the bug that empty lines would be inserted after each prompt,
starting from the first use of ^D.
Readline considers ^D end-of-file even when it's not, and for whatever
reason then adds a \n after BRACK_PASTE_FINI, which results in empty
lines after subsequent prompts.
With readline's internal loop, rl_found_eof gets reset to false, but
users of a custom loop don't get that luxury, and Term::ReadLine::Gnu
doesn't expose rl_found_eof (which was added to readline's API only a
few years ago) to do it manually.
One workaround, used briefly but not committed, would be to disable
bracketed paste.
A better workaround, as implemented by this commit, is to abandon the
custom loop and use readline's blocking call instead.
The eof check has to read a character. It happened to work, but it
was not the right way to check this.
Also added a warning for when someone does "ssh $host revbank" instead
of "ssh -t $host revbank".
This is weird. I'm sure I did test valueless tags. But apparently
between that and committing, the `?` quantifier in the regex got lost,
and I don't know how that happened.
Rationale in UPGRADING.md
It's a big change technically, but converting the format won't be hard
for admins.
There's a compatibility mode with loud warnings in case the file isn't
converted.
It's too buggy; in some edge cases it results in an infinite input loop
with 100% cpu. If you want to restart, use 'restart' instead of eof'ing
the input with ^D.
Wanted to move split_input() to a package for unit testing, thought I'd
move prompt() too since the main executable has become messy, and this
would be a good first step in resolving that.
This won't be relevant for a few years. It's default disabled from Perl
v5.38 which hasn't landed in current Debian stable yet, and since I
intend to support Debian oldstable and stable, there are still many
years ahead of us before this becomes relevant!
Originally, this command didn't have a description parameter. Foo would
use `give xyzzy 10 foo`. Then, a description parameter was added. For
backwards compatibility, if you would enter a username (like `foo` in
this example) in the place of the description, it would finalize the
transaction using that.
However, as the user base grows, several reasonable descriptions exist as
user account names, and that would finalize the transaction under the
wrong user.
It's time to break backward compatibility. If you don't want to leave a
message (it's still optional), that can be done with `x` (like the
`donate` command), or in advanced mode, with `""`.
Because it's likely that people are still very much used to just leaving
the description out, if you enter something that happens to match an
existing username, the input will be rejected.
The current equivalent command line would be `give xyzzy 10 ""; foo` or
`give xyzzy 10 x; foo`. If the `;` (new since v5.0.0) is left out, the
trailing `foo` has to be confirmed with a second press of the enter key.
Not terribly necessary here, because inputs are short, but it's a
good practice. I wish there was a way to just disable backtracking for
the entire regex since this kind of pattern doesn't need any of it.
I can't imagine this to be important but throughout the years it's been
expected by users that "abort" can be quoted and passed to a plugin like
one that prints barcodes.
It's still not possible to pass a literal string `abort` to a follow-up
prompt, leaving this feature only available to advanced users who (hope
to) know what they're doing.
The ancient decision to let undo perform the checkout by itself still
makes sense from a UX perspective, but keeps requiring specific handling
of edge cases.
In this case, the easiest way to deal with trailing input is to just
abort entirely.
Also: updated lib/RevBank/Plugins.pm to import 'isa' and get up to 5.32
level.
