[Fix 72] generator config is not case-sensitive consistent #87
Conversation
There was a problem hiding this comment.
I can't recall why we didn't merge this in earlier, but looking at it again now I've noticed two things:
- I can't seem to replicate the error provided in %config(generator=Internal) does not work but using
internaldoes #72, but do get an error message:
Could not find or check module 'Logos::Generator::Internal::Generator' [THIS MAY BE A PROBLEM!] at /home/lightmann/logos/bin/lib/Logos/Generator.pm line 45.
/home/lightmann/gen/Tweak.x: error: I can't find the 'Internal' Generator!
- It seems like the additional check may be backwards:
can_loadappears to be case-sensitive, so we would want to pop the generator suggestion if the load attempt comes back empty
|
As Ethan explained in the issue thread, you can get the errors with just the It is possible that I'm seeing this issue with a case insensitive filesystem because I'm using MacOS (with changes in this PR): Regarding the check logic, Somewhere in between this and Thunk, the loading of the module does not happen in case insensitive systems and logos shows that "Goto undefined subroutine &main:: at" message. I'll simplify the logic to just make my additions as just an extra piece of code (leave lines 43-45 as they were originally). BTW, the |
Perl's `can_load` is case-insensitive which means that the system can find case variations of generator names but `Thunk` later cannot properly load the module. For now, it detects the typo on the generator name but only warns the user instead of assuming which one they intended, to prevent allowing variations of the names to be used in source code.
uroboro
force-pushed
the
uroboro/fix-72
branch
from
1d233eb
to
d8eb1cd
Compare
There was a problem hiding this comment.
It is possible that I'm seeing this issue with a case insensitive filesystem because I'm using MacOS (with changes in this PR):
Ah, that would explain it. I am unable to repro the goto error on Linux as it is case-sensitive.
BTW, the $Module::Load::Conditional::VERBOSE = 1; at the top might be unnecessary since we always show "....x: error: I can't find the 'Internals' Generator!" message.
I'm not familiar enough with Perl ... does VERBOSE apply everywhere? It seems the only mention of it in the codebase is here.
If we're interested in adjusting the logic to also attempt to advise on generator case for case-sensitive systems, it seems that minimal changes would be required (see comment below). Result with the proposed changes:
%config(generator=internal)#line 1 "generator.x"
%config(generator=Internal)generator.x: error: I can't find the 'Internal' Generator, did you mean 'internal'?
%config(generator=Internals)Could not find or check module 'Logos::Generator::Internals::Generator' [THIS MAY BE A PROBLEM!] at /home/lightmann/logos/bin/lib/Logos/Generator.pm line 59.
generator.x: error: I can't find the 'Internals' Generator!
| if ($generatorDirectoryExists != 1) { | ||
| my %generatorNames = map {lc($_) => $_} @availableGenerators; | ||
| my $possibleGeneratorName = $generatorNames{lc($generatorName)}; | ||
| ::fileError(-1, "I can't find the '$generatorName' Generator, did you mean '$possibleGeneratorName'?"); |
There was a problem hiding this comment.
For case-sensitive OS support:
- Move this new code block above the existing
$GeneratorPackageblock and fall back to the existing fileError if we have no name suggestion.
| ::fileError(-1, "I can't find the '$generatorName' Generator, did you mean '$possibleGeneratorName'?"); | |
| if ($possibleGeneratorName) { | |
| ::fileError(-1, "I can't find the '$generatorName' Generator, did you mean '$possibleGeneratorName'?"); | |
| } |
|
Think this is pretty much ready to go. Did we want to adjust per the comment above or stick to what's pr'd? |
What does this implement/fix? Explain your changes.
Perl's
can_loadis case-insensitive which means that the system can find case variations of generator names butThunklater cannot properly load the module.For now, it detects the typo on the generator name but only warns the user instead of assuming which one they intended, to prevent allowing variations of the names to be used in source code.
Does this close any currently open issues?
#72