Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make rel links checker more precise. #85

Merged
merged 2 commits into from
May 30, 2022
Merged

Conversation

jhchabran
Copy link
Contributor

@jhchabran jhchabran commented May 30, 2022

The predicate used to detect a link pointing to "foo" where the actual
page is at "foo/index.md" was too narrow and prevented to link to other
filetypes such as .csv.

It nows check if the linked file has an extension rather than this
extension just being ".md", which enables to link to CSV files for
example.

Test plan

image

The predicate used to detect a link pointing to "foo" where the actual
page is at "foo/index.md" was too narrow and prevented to link to other
filetypes such as .csv.

It nows check if the linked file has an extension rather than this
extension just being ".md", which enables to link to CSV files for
example.
Copy link

@vrto vrto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small question :)

if isPathOnly && u.Path != "" && !strings.HasSuffix(u.Path, ".md") {
// Require that relative paths link to the actual .md file, i.e not the the "foo" folder in the case of
// of "foo/index.md", so that browsing docs on the file system works.
if isPathOnly && u.Path != "" && filepath.Ext(u.Path) == "" {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have unit tests for these checks or not?

I think filepath.Ext(u.Path) == "" should be filepath.Ext(u.Path) != ""

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vrto that would break the predicate, what we want to catch here are links to "foo" which works because the page is an "index.md". So we only want to focus on paths where we don't have any extension at all.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, this was building a problem case, sorry I misread. All good!

check.go Outdated Show resolved Hide resolved
Co-authored-by: William Bezuidenhout <william.bezuidenhout+github@gmail.com>
@jhchabran jhchabran merged commit 8600fc5 into main May 30, 2022
@jhchabran jhchabran deleted the fix-relative-links-check branch May 30, 2022 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants