Ignore internal urls

[divinerites]

Hello,

Following #168 I made a small patch.
It seems that for this use case, we cannot use the IgnoreURLs because it uses regexp and this is I think too violent for internal urls.

So I added the internal equivalent IgnoreInternalURLs and just done an equal test for url.
Seems to work fine for what I tested.

I also added some tests, but as said I'm a golang noob, so I don't know if those tests suits your quality needs. may be you have to add/reinforce those tests.

Hope this is helpful.
Done with go version go1.16 darwin/amd64

Solve #168

[divinerites]

Hello,

Any news for this PR ? Is there something more that you need ?

[wjdp]

Hey @divinerites sorry for the delay on this. I'm away the rest of this week, will try and get to this next week.

[wjdp]

Hey @divinerites sorry for the delay here.

This change looks good. I was mostly pondering over the new config name as once published these can't really be changed as they end up in in users' configs. I think I'm happy with IgnoreInternalURLs though. Pity I didn't have the forethought needed to name IgnoreURLs better.

---

My other thought is if we could combine IgnoreURLs and IgnoreInternalURLs though this would likely add more complexity: how would you decide what was internal vs external given that an internal doesn't always start with a slash?

Another thought: an improvement to this feature could be understanding the path of the internal URL such that the following works:

• Ignore /misc/js/script.js
• Link to js/script.js inside /misc/page.html is ignored
  For the first pass of this feature though this probably is going too far.

I don't think we should do either of these now, just wonder if you have any thoughts.

---

One thing I'd like to see added before merging is a test of this running on an HTML file. One test where the issue is raised without setting IgnoreInternalURLs and one where it is ignored with the config set. This belongs in check-link_test.go alongside the TestAnchorInternal tests.

Thanks!

[divinerites]

@wjdp thanks for your comments.

• Naming : yes, always difficult when starting things.
• combine IgnoreURLs and IgnoreInternalURLs : I agree that we should let that for a later improvement. Let see how this one is taken & tested by the community. And it is anyway beyond my go skills.

Regarding your request for "test of this running on an HTML file", I'll look if i can, but I'm afraid this is not in my go skills.
Will try to look at it tho.

[wjdp]

@divinerites Have a look at the existing tests, it should be straightforward:

• Find a HTML file / make a file that will fail in fixtures directory
• Add a test (look at the existing ones) that shows that one error is produced with a certain message
• Add another that set the correct config and produces no errors.

There's many examples of this, like:

func TestAnchorDirectoryNoTrailingSlash(t *testing.T) {
	// fails for internal linking to a directory without trailing slash
	hT := tTestFile("fixtures/links/link_directory_without_slash.html")
	tExpectIssueCount(t, hT, 1)
	tExpectIssue(t, hT, "target is a directory, href lacks trailing slash", 1)
}

func TestAnchorDirectoryNoTrailingSlashOption(t *testing.T) {
	// passes for internal linking to a directory without trailing slash when asked
	hT := tTestFileOpts("fixtures/links/link_directory_without_slash.html",
		map[string]interface{}{"IgnoreDirectoryMissingTrailingSlash": true})
	tExpectIssueCount(t, hT, 0)
}

[divinerites]

@wjdp I've done it.
.build.sh seems to work well.
But never properly used tests before. So I guess you have to check if my tests are good and efficient.

[divinerites]

OK. I understood better how to run test. Corrected some wrong syntax.

Tests seems OK except one :

=== RUN   TestIsInternalURLIgnored
    assert.go:73: options_test.go:71: url ignored unexpectedly got: false.
--- FAIL: TestIsInternalURLIgnored (0.00s)

[divinerites]

OK. All test PASS now.

Sorry for being so slow, but this is quite new to me.
Anyway, tests should be good now.

[divinerites]

All tests are now PASSING fine.

[wjdp]

Thanks for adding the tests!

[mtlynch]

Cool, thanks @divinerites and @wjdp!