As promised, here's a PR that is solely concentrated on #21 (handler => destination).
The PR contains 6 commits, described below, so as to ease cherry picking, if desired.
The last state reflects what I propose.
This small change does bring 2 new dependencies though...
require Carp; - RunTime, but only "required" as needed.
use Test::Warn; - For the test suite
Both of those are easy be remove at this time, if we want to.
But they would probably come in handy later on, given the direction where things are headed.
Cheers,
Tabulo[n]?
The GORY DETAILS
General REMARKS
The term handler practically dissapeared from sources
With this change, most occurences of the term handler simply dissapear from the source code.
Where it remains, apart from the deprecation handling and notices, are some of the previous variable names that actually do refer to CODE (e.g. $arg_handler_*).
The map_args() function
You will notice addition of a utility package [Getopt::Long::More::Util] in the code; which currently contains a new little function map_args() which is employed for handling property deprecations and aliases.
I have done it in this way because it's a bit more generic, and also because I already had the bulk of it somewhere :-).
If desired, the body of map_args() can easily be incoporated into ::OptSpec::new(), or rewritten for just handler; but something tells me that we'll have to cater for synonyms of other stuff later on, if we ever wish to support easy migration from the many existing and disparate modules.
That being said, map_args() approach does carry a cost tag of a function call, though.
The COMMITS, explained
Changes BEFORE adding NEW TESTS
57e35fd: Implement #21: handler -> destination
The actual changes to code and docs for implementing #21.
No new tests.
Existing tests still pass, but some cause deprecation warnings for handler, as expected.
++ Adjust previously existing tests so that they use destination instead of handler.
=> Existing tests continue to pass; and this time without any warnings are triggered by the tests themselves.
Still, no new tests.
de9b0b1: Implement #21: handler->destination + adjust tests + readjust docs
++ A few improvements to the docs (mainly concerning destination)
The NEW TESTS
The last three commits are mere variations of the same thing; i.e. adding a couple of tests to ensure that the deprecation itself is properly handled and warned about.
The essence is best captured in the last version, which goes as follows:
subtest "optspec: 'handler' is deprecated -> lives, but warns" => sub {
warnings_exist {
lives_ok { optspec( handler => sub { } ) }
}
[qr/\Whandler\W.*deprecated/],
"optspec: 'handler' is deprecated -> warns";
};
subtest "optspec: Illegal to provide both 'destination' and its deprecated alias 'handler' -> dies" => sub {
local *STDERR = \*STDOUT; # supress the depecation warning before 'die' => Just prettier test output.
dies_ok { optspec(destination => sub {}, handler => sub {} ) };
};
c6c76b2: Add tests specific to the deprecation of "handler"
Hand crafted version of the above tests
58b3590: Use Test::Warnings for some recent tests
Same thing with use [Test::Warnings], hence shorter and prettier,
cfbbf3e: Use Test::Warn for some recent tests
Same thing with use [Test::Warn]; even shorter and prettier,
Hi @Perlancar,
As promised, here's a PR that is solely concentrated on #21 (
handler=>destination).The PR contains 6 commits, described below, so as to ease cherry picking, if desired.
The last state reflects what I propose.
This small change does bring 2 new dependencies though...
require Carp;- RunTime, but only "required" as needed.use Test::Warn;- For the test suiteBoth of those are easy be remove at this time, if we want to. But they would probably come in handy later on, given the direction where things are headed.
Cheers, Tabulo[n]?
The GORY DETAILS
General REMARKS
The term
handlerpractically dissapeared from sourceshandlersimply dissapear from the source code.CODE(e.g.$arg_handler_*).The
map_args()functionYou will notice addition of a utility package [Getopt::Long::More::Util] in the code; which currently contains a new little function
map_args()which is employed for handling property deprecations and aliases.I have done it in this way because it's a bit more generic, and also because I already had the bulk of it somewhere :-).
If desired, the body of
map_args()can easily be incoporated into::OptSpec::new(), or rewritten for justhandler; but something tells me that we'll have to cater for synonyms of other stuff later on, if we ever wish to support easy migration from the many existing and disparate modules.That being said,
map_args()approach does carry a cost tag of a function call, though.The COMMITS, explained
Changes BEFORE adding NEW TESTS
57e35fd: Implement #21: handler -> destination
handler, as expected.9ece031: Implement #21: handler->destination + adjust tests
destinationinstead ofhandler. => Existing tests continue to pass; and this time without any warnings are triggered by the tests themselves.de9b0b1: Implement #21: handler->destination + adjust tests + readjust docs ++ A few improvements to the docs (mainly concerning
destination)The NEW TESTS
The last three commits are mere variations of the same thing; i.e. adding a couple of tests to ensure that the deprecation itself is properly handled and warned about.
The essence is best captured in the last version, which goes as follows:
c6c76b2: Add tests specific to the deprecation of "handler"
58b3590: Use Test::Warnings for some recent tests
use [Test::Warnings], hence shorter and prettier,cfbbf3e: Use Test::Warn for some recent tests
use [Test::Warn]; even shorter and prettier,