Allowing for leading and trailing whitespace in file URI

[jackdeguest]

After adding \s as the special character to be percent-encoded as per rfc3986, the URI::file works fine. See the following test after modification:

#!/usr/local/bin/perl
use v5.10;
use URI::file;
use File::Spec;
my $file = ' hello world ';
my $u = URI::file->new_abs( $file );
my @dirs = File::Spec->splitdir( $u->file( 'Unix' ) );
say $dirs[-1] eq $file ? 'ok' : ( "not ok: '" . $dirs[-1] . "', expected '$file'" );

I also tested with the test units of the distribution and all came out ok.

[oalders]

Fixes #95

[oalders]

Re-opened this in order to start the GH workflow.

[oalders]

There are some failures here related to the workflow. I'm going to fix those tomorrow and then you can pull in your changes and run the tests again.

[jackdeguest]

There are some failures here related to the workflow. I'm going to fix those tomorrow and then you can pull in your changes and run the tests again.

Understood, I will wait for your green light before closing this PR.

[oalders]

Understood, I will wait for your green light before closing this PR.

Just curious as to why you're going to close it?

[jackdeguest]

Understood, I will wait for your green light before closing this PR.

Just curious as to why you're going to close it?

Once you have pulled it in, isn't it useless to keep it around?

[oalders]

Once you have pulled it in, isn't it useless to keep it around?

Yes, but the PR needs to be open in order for us to merge it via the GitHub UI and it will automatically close at the point that it gets merged, so there's nothing for you to do in that regard. You can delete the branch and also the fork once the PR has been merged.

[jackdeguest]

ou can delete the branch and also the fork once the PR has been merged.

Yes, delete is what I meant.

[simbabque]

Could you please also amend the tests? Your test case is good, I think, it can go into t/file.t.

[oalders]

Thanks for the review, @simbabque!

@jackdeguest if you could address that review comment and also rebase to get the latest changes in master, that should allow the most possible test builds to run.

[jackdeguest]

Done. Can you check, because this is the first time I do a rebase. I modified t/file.t, by adding a few extra lines of tests as requested.

[oalders]

Can you check, because this is the first time I do a rebase.

It worked. 😄 A small improvement would be if you in future do something like git pull --rebase origin master. Then it will pull in the latest changes without adding the merge commit, which makes the history a bit easier to follow.

[jackdeguest]

Incidentally, I have to warn you that your test units in t/file.t are a bit simplistic, because URI ≠ URI::file and the tests are based on that assumption.

[oalders]

The guts of that test date back to 1998 cffd342c, so there will be some historical implications to that. We could modernize the tests in a different PR if someone wanted to take that on.