Allow curfile to work in Perl Config files

[haukex]

Summary

Allows __FILE__, __LINE__, and Mojo::File::curfile to work correctly in Perl config files.

Motivation

Allows one to create filenames relative to the current configuration file in the config. For example:

use Mojo::File 'curfile';
{
    migrations_file => curfile->sibling('foo.sql'),
}

[Grinnz]

I'm not totally against this, but the #line hack can be very finicky and I don't think this is really necessary. It's just as easy to store paths relative to your app home dir in the config file, and have the reader append it to the homedir.

[haukex]

@Grinnz

the #line hack can be very finicky

Hm, if the filename contains a newline, that might be a problem, so I "fixed" that in the updated commit (s/\n/\\n/g, not pretty...). Is there anything else?

[haukex]

I was surprised by the failure on Windows:

#   Failed test 'right value'
#   at t\mojolicious\perl_config_lite_app.t line 14.
#     Structures begin differing at:
#          $got->{file} = 'C:/projects/mojo/t/mojolicious/perl_config_lite_app.conf'
#     $expected->{file} = 'C:\projects\mojo\t\mojolicious\perl_config_lite_app.conf'
# Looks like you failed 1 test of 1.

I hope my attempted fix of adding ->to_abs works - rel2abs calls canonpath for cleanup. Edit: Works now.

[Grinnz]

Spaces in the filename also break the #line directive, but this will just make __FILE__ not work in that case and not break anything else.

[Grinnz]

Spaces in a filename work if it's double quoted, but then double quotes in the filename don't work. It's really just not well designed for generated code.

[haukex]

@Grinnz

Spaces in the filename also break the #line directive, but this will just make __FILE__ not work in that case and not break anything else.

Spaces in a filename work if it's double quoted, but then double quotes in the filename don't work. It's really just not well designed for generated code.

Hrm, that is indeed not great. On the one hand, I could just add quotes around the filename and hope nothing breaks - but on the other hand, I guess the "defensive" way to code it would be to not include the #line directive if it's going to break, so that's what I've done in the latest commit. Still not super pretty, unfortunately...

It's just as easy to store paths relative to your app home dir in the config file, and have the reader append it to the homedir.

Background for why I suggested this in the first place: I'd like to use the same configuration in a non-Mojo script by evaling it, and it may be in a different path. (And of course adding the #line makes error messages nicer.)

[haukex]

I looked at Perl's code and indeed there is no way to escape double quotes in #line, but it looks like any other characters (except \n) should work.

So, I've simplified the code changes accordingly, added more tests to cover the special cases, and added a line in the documentation.

I feel like I've done everything I can for now - I hope this passes muster.

[mergify]

This pull request is now in conflicts. Could you fix it @haukex? 🙏

[kraih]

While i might vote in favour of the feature itself if done in the most minimalistic way possible, there is a lot of special cases in this PR i do not agree with. So 👎 from me on this specific implementation of the feature.

[kraih]

Or to be more precise, there is already #line use that should be emulated in Mojo::Template.

[haukex]

@kraih
Yes, originally it did contain more special cases than necessary (the commits should be squash merged, or I could open a new PR if desired), but now the only thing that's left is \n and ", just like in Mojo::Template - or do you mean other special cases?
I'm not sure I understand your comment in regards to #line in Mojo::Template, are you suggesting code re-use between the two?

[jberger]

I personally don't love that the file is evaluated at all and I've been tending to use JSON and YAML config wherever possible. Meanwhile if your case for it is

Background for why I suggested this in the first place: I'd like to use the same configuration in a non-Mojo script by evaling it, and it may be in a different path.

then I really disagree. My understanding of the reasoning for evaluating was to give access to the app itself. In a non-mojo context, the app having no meaning, you're using a config file as a generic runnable file. If you need that I'd suggest simply doing so, do $file the file on your own and load it into $app->config at startup. The config loader module isn't magic and apart from running the file it essentially does the same thing. 👎 from me unfortunately

[marcusramberg]

I'm also against adding this functionality to the config. 👎

[haukex]

Thanks for the feedback everyone.

[kraih]

In case someone stumbles over this in the future, the current solution is:

{
    migrations_file => app->home->child('foo.sql'),
}

Without the need for imports.