Skip to content

Missing link in ice-breaker/about.md #478

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

Closed
CreepySkeleton opened this issue Oct 23, 2019 · 15 comments
Closed

Missing link in ice-breaker/about.md #478

CreepySkeleton opened this issue Oct 23, 2019 · 15 comments

Comments

@CreepySkeleton
Copy link
Contributor

I think [claim the issue] is supposed to lead somewhere, but it doesn't

@amanjeev
Copy link
Member

Can you please elaborate? Are you a member you wants to have the issue assigned yourself?

@mark-i-m
Copy link
Member

@amanjeev I think they mean that the [claim this issue] text in https://github.com/rust-lang/rustc-guide/blob/master/src/ice-breaker/about.md is likely supposed to be a link but was not checked.

@Michael-F-Bryan is there any way to make the link checker check these sorts of links?

@spastorino @nikomatsakis What was this supposed to link to?

@CreepySkeleton
Copy link
Contributor Author

@mark-i-m

I think they mean that the [claim this issue] text in https://github.com/rust-lang/rustc-guide/blob/master/src/ice-breaker/about.md is likely supposed to be a link but was not checked.

Yes, exactly.

@Michael-F-Bryan is there any way to make the link checker check these sorts of links?

Yes, I'm about to send such a PR.

@spastorino
Copy link
Member

I guess that should have been https://github.com/rust-lang/triagebot/wiki/Assignment or just saying claim this issue by commenting with @rustbot claim. Unsure what exactly the intention if @nikomatsakis was though.

@CreepySkeleton
Copy link
Contributor Author

Hm, it looks like there's checking via mdbook-linkcheck but it doesn't check for situations like this.

@ehuss
Copy link
Contributor

ehuss commented Oct 23, 2019

is there any way to make the link checker check these sorts of links?

It is possible, but difficult. pulldown-cmark has a broken link callback, but it doesn't work very well (it is global, and does not tell you where the link is). I've experimented with this in mdBook, and found a ton of false positives, because [foo] is valid markdown, and is intentionally allowed. It might have to be a pedantic-style check, due to the high false positive risk.

Fixing the callback is tracked in pulldown-cmark/pulldown-cmark#165 and pulldown-cmark/pulldown-cmark#373. It is also a blocker for updating pulldown-cmark in rustdoc, so it would be nice to get some movement on it. Someone just needs to propose a good API that could work with pulldown-cmark's architecture.

There's a similar issue of broken references are not validated (pulldown-cmark/pulldown-cmark#217).

@CreepySkeleton
Copy link
Contributor Author

OK, then we might set up a convention not to use any [link] links and replace them with [link][]. What do you think?

@ehuss
Copy link
Contributor

ehuss commented Oct 23, 2019

I don't see how a collapsed reference link would change anything? I think it would also be difficult (and tedious) to enforce conventions.

What I would encourage is for people to use "View file" link in GitHub PR reviews and read the rendered output to make sure any markdown rendering issues are caught. GitHub's renderer is usually close enough to mdBook's.

@mark-i-m
Copy link
Member

Yes, I think for now, the easiest way to do this would be to enforce it in the review process.

@Michael-F-Bryan
Copy link
Contributor

@mark-i-m I've added initial support for this in mdbook-linkcheck. At the moment pulldown-cmark doesn't provide the byte index of the broken link, so I've implemented a small best-effort hack using string searching.

This sort of incomplete link is valid markdown, so I'm only emitting a warning:

Screenshot from 2019-10-24 14-58-35

I'll probably create a fail-on-warnings flag for use in CI before making another release.

@mark-i-m
Copy link
Member

@Michael-F-Bryan Awesome! Is that on the linkcheck 0.4.0? Is there a setting to enable it?

@mark-i-m
Copy link
Member

I'll probably create a fail-on-warnings flag for use in CI before making another release.

That would be awesome.

@spastorino
Copy link
Member

Just asked @nikomatsakis and indeed he aimed to link to https://github.com/rust-lang/triagebot/wiki/Assignment/. @CreepySkeleton would you like to provide a PR?.

@CreepySkeleton
Copy link
Contributor Author

CreepySkeleton commented Oct 24, 2019

I don't see how a collapsed reference link would change anything? I think it would also be difficult (and tedious) to enforce conventions.

The intention here is to separate [regular text enclosed with square braces] and [collapsed link][] use cases. I believe it would also be easier to automatically check such collapsed links, @Michael-F-Bryan any thoughts?

@spastorino
Copy link
Member

After #480 this is fixed. Closing this issue. Can you open a new issue or a PR when you have a solution for the link checking idea?.

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

No branches or pull requests

6 participants