feat(base-url)!: disallow relative local base to avoid confusion

[katrinafyi]

the previous behaviour accepted relative directory paths as bases but this led to later InvalidBaseJoin errors, because relative bases cannot be used to join relative URLs. this means that relative local bases were only useful for resolving root-relative links, and were confusingly problematic with ordinary relative links.

see #1574 which talks about errors when passing --base ../network-documentation/ or, in a later comment, --base build. it is my opinion that it is better to fail earlier in these cases, so the user is not hit with mysterious InvalidBaseJoin.

also, the previous behaviour would parse something like --base-url google.com to a local base pointing to ./google.com. this would also lead to downstream errors or incorrect URLs and it's better to guard against this.

after this pr, trying to use a non-absolute local base will be a CLI parsing error:

error: invalid value 'build' for '--base-url <BASE_URL>': Error with
base dir `build` : Base must either be a URL (with scheme) or an
absolute path. Alternatively, if you want to resolve root-relative links
in local files, see `--root-dir`.

in a slightly opinionated touch, i've mentioned --root-dir in the error message, since i think --root-dir is more suitable for most use cases where people try to use relative local bases. this agrees with later comments in #1574.

however, this does make the error message quite long. so i'm happy to take on feedback and changes about this. also, i'm not quite sure which conventional commit verb to use in this PR title. i've put "feat" for now.

also, an alernative approach to think about is to ban "local" bases altogether and require use of file:/// for local paths. this would unify the argument under the URL format and maybe help prod people towards --root-dir for local paths.

also, another alternative instead of doing path.is_absolute() is to make a Local base then try to .join() with it and ensure the join succeeds. this will more precisely guard against join failures (and hence, InvalidBaseJoin errors)

this pr implements (1) in my outline to reduce InvalidBaseJoin confusion, as described in #1624 (comment)

[mre]

Do you think we should add a few examples or reference to the docs here?

[katrinafyi]

For adding examples, I think that this message location isn't quite suitable, the current message is already relatively large. Maybe a link to the docs would be okay, but idk which docs. The base url page is not super informative.

Maybe it could mention that more info is in --help? But that's also kind of obvious.

[mre]

You're right.

The base url page is definitely not great and needs a makeover at some point. Besides, hardcoding website URLs is probably not a good idea. Would be quite ironic if that link breaks in the future. 😆

Setting up the base url and root dir are probably the trickiest things to get right from a user's perspective, so in this case mentioning --help might be a good idea.

[katrinafyi]

I've made some changes in response to the review comments, but is the CI workflow working? It's failed in a setup step twice in a row.

Also, I'm wondering if this PR's approach is a good idea. It's adding a message to TryFrom<&str> for Base that mentions command-line flags, but this is part of lychee-lib and could be used entirely without the CLI.

[mre]

is the CI workflow working? It's failed in a setup step twice in a row.

Yeah, I noticed that in a few other projects as well.
#1862

[mre]

Also, I'm wondering if this PR's approach is a good idea.
The approach is fine, I think. Or at least it's a step into the right direction to avoid confusion around these options.

It's adding a message to TryFrom<&str> for Base that mentions command-line flags, but this is part of lychee-lib and could be used entirely without the CLI.

Yeah, that's indeed not great. Sorry for not noticing this sooner and leading you astray. Maybe we can instead add more .context whereever this TryFrom gets called in the CLI for a cleaner separation of concerns.

[mre]

#1862 is merged so you can rebase.

[katrinafyi]

Thanks for the fix and the hint about context. I've made the changes and it makes a bit more sense now. See the commit comment, though.

[mre]

This looks great now! Unless you'd like to try the suggestion in the comment of which I'm not sure if it will work (and you've possibly tried it already), we can go ahead and merge this. Great progress.

[katrinafyi]

I've tweaked the --base-url help text to read a bit better from top to bottom. I'm happy for it to be merged anytime now :)

[mre]

Good work, thanks

[thomas-zahner]

Documentation is outdated:

      - name: Link Checker
        id: lychee
        uses: lycheeverse/lychee-action@v2
        with:
          args: --base-url dist --exclude-all-private dist
          fail: false

Will now lead to an error as expected. @katrinafyi how would you fix this?

I tried

          args: --dump --base-url "$(pwd)/dist/" --exclude-all-private dist

but this doesn't work. I guess this is a GitHub workflow related issue. How would you be able to access the absolute path?
I've also tried ${GITHUB_WORKSPACE} and ${{ github.workspace }} without success.

See lycheeverse/lycheeverse.github.io#114 and https://github.com/lycheeverse/lycheeverse.github.io/actions/runs/19075734005/job/54490637825?pr=114

[katrinafyi]

@thomas-zahner I started a fix here but there's other (unrelated) issues to do with how existing links are excluded: lycheeverse/lycheeverse.github.io#111

In hindsight, maybe we should've been more careful with this change 😅 even if it was slightly buggy, i think base-url was very widely used...

[thomas-zahner]

Ah I've missed that one. Thank you. Yes maybe we were a bit quick to release that but at the same time we're still not on v1 so I think it's okay if the docs are up to date.