Remove :CHECK-DNS-P and DNS lookups (pending)

[shamazmazum]

Hello! In the commit 377e9ab a call to valid-url-p was replaced with valid-tld-p in the initialize-instance for new-url-query which means that queries like github.com/atlas-engineer are always opened in a search engine ((valid-tld-p "github.com/atals-engineer") is nil). After that commit check-dns-p is also never passed to valid-url-p, so I remove it.

The only place where check-dns-p is used is in initialize-instance itself. This argument was introduced by me in 485f042 to deal with long latency times when using set-url. I cannot remember why I added a check for check-dns-p in initialize-instance and now I remove it in this commit.

[shamazmazum]

Also isn't it a good idea to remove dns lookups completely?

[aadcg]

@shamazmazum, in commit 377e9ab I had to preserve the API so that the fix would land in the 3-series releases. I had the same urge to remove DNS lookups as you. This is a good opportunity to change the API so thank you for bringing it up.

Remove DNS lookups

From my cursory investigation, we brought them in commit fc25179113b6d419dbb2d35b1597b6365886855d. There must be a better way to solve the problem. If we remove check-dns-p from valid-url-p, some tests may fail so please check it. Still, if you can remove them and keep the functionality, I'm in strong favor.

Implicit bug report

queries like github.com/atlas-engineer are always opened in a search engine ((valid-tld-p "github.com/atals-engineer") is nil)

This is a major bug, and it's my fault that I didn't take it into account. We need a fix that is compatible with the 3-series API. It may be slightly off-topic for this PR so I'd suggest opening an issue about it or sending a patch.

Other quirks

Perhaps somehow related to DNS lookups, there is also another less important issue that would be nice to fix.

[shamazmazum]

@aadcg I don't quite get what you mean by "a fix that is compatible with the 3-series API". Do you mean that some third-party software is supposed to call input->queries or make-completion-query with :check-dns-p from which I removed that argument? Btw, they are not even exported. With exception of these two functions API is the same.

If you wish a smaller fix which just fixes a navigation to URLs like "example.com/foo", I can just fix an erroneous conditional in initialize-instance but leave :check-dns-p be. I thought it was better to remove it because it is not needed in buffer.lisp at all.

So what should I do?

[aadcg]

I don't quite get what you mean by "a fix that is compatible with the 3-series API". Do you mean that some third-party software is supposed to call input->queries or make-completion-query with :check-dns-p from which I removed that argument? Btw, they are not even exported. With exception of these two functions API is the same.

Technically, you're correct, but I'd rather lean on the side of being conservative and avoid changes to the function signature on releases that don't bump the major version.

So what should I do?

In this PR, let's address removing DNS lookups, and we can make arbitrary changes in the codebase.

In the separate one, let's do exactly what you suggest:

a smaller fix which just fixes a navigation to URLs like "example.com/foo", I can just fix an erroneous conditional in initialize-instance but leave :check-dns-p be.

I'd suggest doing the small fix above first, and then working on this PR that gets rid of DNS lookups. Does that make sense?

[shamazmazum]

Yes, I've opened a new PR which fixes just the navigation.

[aadcg]

Done, see https://github.com/atlas-engineer/nyxt/pull/3385#issuecomment-2061088011.