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

Add code to support %DOMAIN% tag replacer #1371

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

nrathaus
Copy link

Description

What will it do?

If this PR will fix an issue, please address it:
#1313

if it finds a %DOMAIN% tag inside the dictionary, it will replace it with the hostname field of the URL - if multiple URLs are provided it will replace it multiple times

@maurosoria
Copy link
Owner

Hi! Glad to meet you @nrathaus ! How you doing?

I like it but I wonder, for example if type www.example.com:

  • Shouldn't try example.com?
  • Or just example?

For example, given the case "%DOMAIN%.zip":

@nrathaus
Copy link
Author

@maurosoria I added the iterator so that it will go through the components of the domain

Let me know if this is what you were expecting

@maurosoria
Copy link
Owner

Hi again!

I see you got me but there are 3 issues with your commit:

  • What happens when a line has both %EXT% or %DOMAIN% in the same line (or any other future tag)? I see in this commit you only allow one tag per line
  • domain can be parsed before entering the loop in line 125 (also you can use tldrextract or a regexp to parse the domain)
  • Same behaviour should be applied at line 196

Thanks for your quick response. If you feel I am too demanding please tell me. I Like your patch (I also like other ones like %YEAR%!!!)!

@nrathaus
Copy link
Author

I did a one tag replacer because EXT had a similar design, to support multiple tags - we would need to rewrite the whole function used - I am ok with doing that

@nrathaus
Copy link
Author

Regarding tldrextract, I didn't want to introduce another library - we don't actually need this in my opinion, just dot breaking is sufficient for this dirsearch improvement

I added additional code, didn't get a chance to test it yet - will try to do it later

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.

None yet

2 participants