markdown: detect valid url #113

laysauchoa · 2020-09-26T18:57:08Z

Steps:

[https://ahoi.io](https://ahoi.io)
[fun](../fun.html)
[struct Foo](super::Foo)

Signed-off-by: Laysa Uchoa laysa.uchoa@gmail.com

What does this PR accomplish?

🩹 Bug Fix

Closes # #44

Changes proposed by this PR:

detect valid urls in [] so spell checkings can be ignored in that span https://ahoi.io

📜 Checklist

Works on the ./demo sub directory
Test coverage is excellent and passes
Documentation is thorough

in case of valid urls, spell checking can ignore in case of valid ones. Signed-off-by: Laysa Uchoa <laysa.uchoa@gmail.com>

laysauchoa · 2020-09-27T16:42:09Z

src/main.rs

@@ -110,6 +111,7 @@ struct Args {
    flag_skip_readme: bool,
    flag_code: u8,
    flag_stdout: bool,
+    flag_reachable_link: bool,


cc @drahnr
silly question, where can I pick up the result of what is passed in the flag? Can I pick up in the markdown file?

Can you elaborate where you need what information? I don't quite understand what you are asking for.

The Args struct contains all the commandline parameters. From there on you have to thread 🧵 / stitch 🪡 it through to the place you need it. If there are already other arguments passed through, consider adding a per component argument struct.

I hope that covers it :) if not, please elaborate on you question ❓

I actually do not know how to or where should I save the boolean value of this flag from main to markdown file.
Should I do something like this? or how can I do: save the boolean value of the flag so I can do actions in markdown about whether the links should be checked for Get request or not?

lazy_static! { static ref USERS: HashMap<&'static str, Vec<&'static str>> = { let mut map = HashMap::new(); map.insert("Loe", vec!["user", "user"]); map }; } fn show_user(name: &str) { let access = USERS.get(name); println!("{}: {:?}", name, access); }

Thank you in advance

markdown file

I assume you mean propagate the bool from main.rs / fn main to markdown.rs.

Just looked into the code now, probably should have done that earlier.

Soo, this is indeed a bit iffy. There are three options (see detailed discussion underneath):

fiddle the bool through to erase_cmark

make the whole configuration a global singleton

make the link destination check a separate, indepedent checker that implements trait Checker

I would recommend option 3., since markdown parsing is pretty cheap compared to the http lookup (even with smid yet to be enabled), and also compared to the dictionary lookup (see #57 for the flamegraph)

Option 1. is feasible, but would mix concerns rather unpleasantly - a mashup of a feature and a parsing layer - not so good and entangles the code quite a bit.

Option 2. is something for a server application / service mostly done for ad-hoc re-configuration, which would work too, but is somewhat special and makes testing harder, so I think the trade of is simply not given here.

Very good question!

I did not really follow how adding the Checker trait will help me to have the desired flag in markdown file.

This is an example of the config I found in the Hunspell checker like here:
https://github.com/drahnr/cargo-spellcheck/blob/master/src/checker/hunspell.rs#L118

{ lang: Some("en_US"), search_dirs: SearchDirs(Some(["/home/tmhdev/Documents/cargo-spellcheck/.config", "/usr/share/hunspell"])), extra_dictionaries: Some(["/home/tmhdev/Documents/cargo-spellcheck/.config/lingo.dic"]), quirks: Some(Quirks { transform_regex: Some([WrappedRegex(^'([^\s])'$), WrappedRegex(^[0-9]+x$), WrappedRegex(^\#[0-9]+$), WrappedRegex(^[0-9]+$), WrappedRegex(^.+\+$)]), allow_concatenation: Some(true), allow_dashes: None }) }

Also, I am confused if you meant that this flag reachable_link should not be a boolean but rather be passed to:
--checkers=<checkers> Calculate the intersection between

I am confused about this step. Probably because I lack to understand how other parts of the project work.

I did not get notifed of this comment 🤨 maybe one of the plenty gh outages swallowed that notification, maybe I screwed up.

The issue here is, that currently the markdown is a layer in the general processing of input - which as of today - has no knowledge of any configuration, it's in it's core a pseudo-stream based pipeline with origin tracking.

Now if you would implement this with the trait Checker you would be called straight from main like Hunspell and LanguageTool (and soon Reflow) are, each with their defined params. So if you move away from being just a knot or layer in the input processing, but actually a consumer of the output of those layers, you can operate on the "raw" chunks (with markdown included) and do the checks right there.

Hopefully I can find a few minutes to generate some documentation images for #54 to clarify the whole flow a bit.

drahnr

Minuscule nitpicks only 👍

src/documentation/markdown.rs

src/main.rs

Co-authored-by: Bernhard Schuster <bernhard@ahoi.io>

…pellcheck into laysa-cmark-links-handled

drahnr · 2020-10-09T10:53:29Z

I'd like to release a 1.0.0 rather soon~ish (end of October~ish), and I'd love to include this feature with that :) Do you think you that is enough time to complete it until then? Is there anything you need help with?

Signed-off-by: Laysa Uchoa <laysa.uchoa@gmail.com>

laysauchoa · 2020-10-16T13:46:16Z

Unfortunately, closing it as I did not figure out how to bring flags to other files needed for this PR.

drahnr · 2020-10-16T13:55:19Z

You are so close, it's just one last refactor with the trait Checker to make it all work ( 🤞 ) but if you feel this is becoming a chore, I can take over from here. Whatever you prefer.

drahnr · 2020-10-18T13:30:05Z

@laysauchoa I went forward and added the integration with the configuration file according to Option 1) described earlier - it turned out less messy than anticipated.
Do you want to tackle the rest or should I take it over in its enterty?

laysauchoa · 2020-10-18T14:28:42Z

@laysauchoa I went forward and added the integration with the configuration file according to Option 1) described earlier - it turned out less messy than anticipated.
Do you want to tackle the rest or should I take it over in its enterty?

thanks, @drahnr I had no idea how to do this. I understood that the flag will be link_check. I will try to continue from here. Thanks a lot!

laysauchoa · 2020-10-26T16:36:01Z

@laysauchoa I went forward and added the integration with the configuration file according to Option 1) described earlier - it turned out less messy than anticipated.
Do you want to tackle the rest or should I take it over in its enterty?

Would it be possible to update the readme how to use the integration for the flag added it? I put a place holder because I am still missing the value for config.

src/documentation/markdown.rs

drahnr · 2020-10-26T16:58:33Z

src/documentation/markdown.rs

@@ -61,6 +62,14 @@ impl<'a> PlainOverlay<'a> {
    fn valid_url(url: &str) -> bool {
        url::Url::parse(url).is_ok()
    }
+    fn reachable_link(url: &str) -> bool {
+        use reqwest;
+        let response = reqwest::blocking::get(url).unwrap();


If we want this to remain blocking, which I think makes sense, we should think about parallelizing work per document eventuall (separate PR).

We should also track which url we already checked, we don't want to spam a server with requests and on the other side cause the user to wait for an extended time for not much benefit.

drahnr · 2020-10-26T17:07:09Z

@laysauchoa I went forward and added the integration with the configuration file according to Option 1) described earlier - it turned out less messy than anticipated.
Do you want to tackle the rest or should I take it over in its enterty?

Would it be possible to update the readme how to use the integration for the flag added it? I put a place holder because I am still missing the value for config.

I think in this case you either need to add &self to fn extract_plain_with_mapping (here~ish

cargo-spellcheck/src/documentation/markdown.rs

Line 80 in 52ddd52

    
           fn extract_plain_with_mapping(cmark: &str) -> (String, IndexMap<Range, Range>) {

) as argument, or feed out the resulting unreachable websites via a suggestion variant, and then, later on filtering that. Does that make sense?

drahnr · 2020-11-14T12:14:05Z

@laysauchoa I merged all the giant reflow stuff so I'll go ahead and rebase this.

Could you outline which work items are still open? Is anything blocking you right now?

drahnr · 2020-12-02T07:18:59Z

We could take some code from lychee, which does all the link checking pretty nicely already :)

drahnr · 2021-07-30T15:18:02Z

It's been a while :) I think we should come to an end and attempt to push it over the finish line, or define the topic to be out of scope for cargo-spellcheck unless somebody else wants to pick it up.

drahnr · 2022-05-04T06:36:21Z

Unfortunately closing, would love to see this eventually

markdown: detect valid url

cdef99c

in case of valid urls, spell checking can ignore in case of valid ones. Signed-off-by: Laysa Uchoa <laysa.uchoa@gmail.com>

laysauchoa force-pushed the laysa-cmark-links-handled branch from 78e3ebf to cdef99c Compare September 26, 2020 19:02

laysauchoa requested a review from drahnr September 26, 2020 19:41

laysauchoa self-assigned this Sep 26, 2020

markdown: rename variable previous to current_link_type

c804b7d

laysauchoa marked this pull request as draft September 27, 2020 10:57

command line: add flag for reachable link

c01dfdc

laysauchoa commented Sep 27, 2020

View reviewed changes

drahnr reviewed Sep 28, 2020

View reviewed changes

drahnr mentioned this pull request Sep 28, 2020

accept arbitrary urls #110

Open

laysauchoa and others added 3 commits September 29, 2020 08:40

markdown: use is_ok() instead of match

1c2c68a

Co-authored-by: Bernhard Schuster <bernhard@ahoi.io>

markdown/fix: rename variable and add docstrings

3512f75

Merge branch 'laysa-cmark-links-handled' of github.com:drahnr/cargo-s…

04c8d3a

…pellcheck into laysa-cmark-links-handled

markdown/fix: use if instead of match and rename variables

a5d07c7

Signed-off-by: Laysa Uchoa <laysa.uchoa@gmail.com>

laysauchoa closed this Oct 16, 2020

drahnr reopened this Oct 16, 2020

feat/config: integrate common mark validation flags

92d6ec9

drahnr added enhancement 🦚 New feature or request hacktoberfest labels Oct 18, 2020

laysauchoa added 2 commits October 26, 2020 17:25

cargo: use reqwest crate

9032b74

fix/markdown: check if link is reachable

52ddd52

laysauchoa marked this pull request as ready for review October 26, 2020 16:34

laysauchoa requested a review from drahnr October 26, 2020 16:36

drahnr reviewed Oct 26, 2020

View reviewed changes

src/documentation/markdown.rs Outdated Show resolved Hide resolved

drahnr reviewed Oct 26, 2020

View reviewed changes

refactor: minor simplification

564f68d

drahnr changed the title ~~[wip] markdown: detect valid url~~ markdown: detect valid url Nov 3, 2020

drahnr removed the hacktoberfest 🍻 label Nov 8, 2020

drahnr mentioned this pull request Dec 4, 2020

Expose the link check as library lycheeverse/lychee#64

Closed

drahnr mentioned this pull request Dec 15, 2020

add link check with lychee #131

Closed

3 tasks

drahnr mentioned this pull request Apr 27, 2021

linkcheck #175

Open

3 tasks

drahnr added the stale 😐 PRs and issues that need some additional work to make it across the finishline label Aug 18, 2021

drahnr closed this May 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

markdown: detect valid url #113

markdown: detect valid url #113

laysauchoa commented Sep 26, 2020 •

edited by drahnr

laysauchoa Sep 27, 2020

drahnr Sep 28, 2020

laysauchoa Oct 5, 2020

drahnr Oct 7, 2020 •

edited

laysauchoa Oct 12, 2020

drahnr Oct 16, 2020 •

edited

drahnr left a comment

drahnr commented Oct 9, 2020 •

edited

laysauchoa commented Oct 16, 2020

drahnr commented Oct 16, 2020

drahnr commented Oct 18, 2020

laysauchoa commented Oct 18, 2020 •

edited

laysauchoa commented Oct 26, 2020

drahnr Oct 26, 2020

drahnr Nov 10, 2020

drahnr commented Oct 26, 2020 •

edited

drahnr commented Nov 14, 2020

drahnr commented Dec 2, 2020

drahnr commented Jul 30, 2021

drahnr commented May 4, 2022

markdown: detect valid url #113

markdown: detect valid url #113

Conversation

laysauchoa commented Sep 26, 2020 • edited by drahnr

What does this PR accomplish?

Changes proposed by this PR:

📜 Checklist

laysauchoa Sep 27, 2020

Choose a reason for hiding this comment

drahnr Sep 28, 2020

Choose a reason for hiding this comment

laysauchoa Oct 5, 2020

Choose a reason for hiding this comment

drahnr Oct 7, 2020 • edited

Choose a reason for hiding this comment

laysauchoa Oct 12, 2020

Choose a reason for hiding this comment

drahnr Oct 16, 2020 • edited

Choose a reason for hiding this comment

drahnr left a comment

Choose a reason for hiding this comment

drahnr commented Oct 9, 2020 • edited

laysauchoa commented Oct 16, 2020

drahnr commented Oct 16, 2020

drahnr commented Oct 18, 2020

laysauchoa commented Oct 18, 2020 • edited

laysauchoa commented Oct 26, 2020

drahnr Oct 26, 2020

Choose a reason for hiding this comment

drahnr Nov 10, 2020

Choose a reason for hiding this comment

drahnr commented Oct 26, 2020 • edited

drahnr commented Nov 14, 2020

drahnr commented Dec 2, 2020

drahnr commented Jul 30, 2021

drahnr commented May 4, 2022

laysauchoa commented Sep 26, 2020 •

edited by drahnr

drahnr Oct 7, 2020 •

edited

drahnr Oct 16, 2020 •

edited

drahnr commented Oct 9, 2020 •

edited

laysauchoa commented Oct 18, 2020 •

edited

drahnr commented Oct 26, 2020 •

edited