Fix #3791 Add adverbs to --doc compiler module definition #3807
Conversation
When compiler called with --doc=XXXX only the module Pod::To::XXX is loaded. --doc=XXXX:ver<1.2.3>:auth<Perl 6> will cause an error. This patch recognises :ver<>:auth<>:api<> combinations. In addition, it allows for --doc=Some::Arbitrary::Renderer to be checked. If no such module exists, then Pod::To::Some::Arbitrary::Renderer is attempted to be loaded.
|
fwiw I’ve never liked arbitrary namespace prefixes in raku, especially in combination with full namespaces. Things like what to load when passed ‘Foo’ and both Pod::To::Foo and Foo exists (picking one or the other is easy, but makes things inconsistent for the sake of saving a few keystrokes). |
| my $rx := / | ||
| ^ | ||
| $<name> = ( [ \w | '::' ] + ) | ||
| [ ':' $<part> = ( [ 'ver' | 'auth' | 'api' ] ) |
There was a problem hiding this comment.
This would match "::a::::bbbbb". I think rakudo has a canonical parser for this, can you please look this up?
Also :ver<0.1>:ver<33>. You might end up with something you can't instantiate and fail later.
There was a problem hiding this comment.
a) Don;t think so because : must be followed by ver/auth/api. : (::bbbbb) and :: (:bbbbb) would fail.
b) I am certain it does, but where? and how could I look this up? nqp documentation is not so good. I have looked at everything on nqp site.
src/Perl6/Actions.nqp
Outdated
| while $i < $e { nqp::bindkey( %opts, ~ @parts[$i], ~ @vals[$i++] ) }; | ||
|
|
||
| my $renderer := ~ nqp::atkey($s,'name'); | ||
| my $module; |
There was a problem hiding this comment.
You sure that what atkey remembers is something other than a string?
There was a problem hiding this comment.
Yes. atkey returned matches for the parts and vals. That caused failures, so the ~ was needed there. I added for name, just incase.
src/Perl6/Actions.nqp
Outdated
| $module := $*W.load_module($/, $renderer, %opts, $block); | ||
| CATCH { | ||
| $renderer := 'Pod::To::' ~ nqp::atkey($s,'name'); | ||
| $module := $*W.load_module($/, $renderer, %opts, $block); |
There was a problem hiding this comment.
What if this fails? Also, you're introducing the ambiguity that @ugexe has pointed out, which means that now Any::Renderer::I::Want could be used here. Do we want this?
There was a problem hiding this comment.
JJ I think you misunderstood @ugexe, or perhaps I do.
He was saying, like I was saying, the argument to --doc= should be a fully qualified module.
But the API requires (as Damian Conway stated in his original POD specification) that it should be --doc=HTML.
@AlexDaniel pointed out that =HTML already exists and is documented.
So my suggestion is to test for the fully qualified name, but then test for a Pod::To::xxx name.
If that also fails, then the compiler fails, as it does now. There is no default specified. It could default to Pod::To::Text but that is NOT yet specified anywhere.
|
I think treating the module name as fully qualified and falling back on trying with a Pod::To:: prefix to stay compatible with documented behaviour is a good strategy. The alternative would be to treat some names as special. |
|
@finanalyst should this PR still remain open? |
|
I'm unsure as to the state of this PR. Does this need more work? Can it be merged? Should it be closed? |
|
I would need to check it a bit more extensively. Probably some Rakudo tests would need to be written too, since it's introducing new behavior. |
When compiler called with --doc=XXXX only the module Pod::To::XXX is loaded.
--doc=XXXX:ver<1.2.3>:auth<Perl 6> will cause an error.
This patch recognises :ver<>:auth<>:api<> combinations. In addition, it allows for --doc=Some::Arbitrary::Renderer to be
checked. If no such module exists, then Pod::To::Some::Arbitrary::Renderer is attempted to be loaded.