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

Port list download code to JS #25

Merged
merged 9 commits into from Sep 18, 2023
Merged

Conversation

hlqviet
Copy link
Contributor

@hlqviet hlqviet commented Sep 17, 2023

I wish I could remove all the shell scripts but for the sake of compatibility 🥲

@mrrfv
Copy link
Owner

mrrfv commented Sep 17, 2023

The names whitelist.csv and input.csv shouldn't be completely removed for compatibility with older versions (maybe add a fallback?). And I think it'll be a good idea to just go all-in on Node.js for allow/blocklist downloads and replace the contents of get_recommended_*.sh with node [script].js commands so the lists can be downloaded on any system, including Windows.

@hlqviet
Copy link
Contributor Author

hlqviet commented Sep 17, 2023

Hmm... Let me revert the name changes then.

Edit: But wait, aren't they generated by the shell scripts and we already updated them?

@mrrfv
Copy link
Owner

mrrfv commented Sep 17, 2023

Fair enough. I didn't notice that. There's still a probability that the script's going to break for people using their own lists though.

Hmm... Let me revert the name changes then.

No need to revert them, just dynamically adjust the name of files being used based on which one exists. You can use fs.existsSync() for that.

@hlqviet
Copy link
Contributor Author

hlqviet commented Sep 17, 2023

So here it is 😁

@hlqviet
Copy link
Contributor Author

hlqviet commented Sep 17, 2023

May want to add to the README that these scripts support hosts, domain and adblock formats but work best with wildcard domain (without asterisk) format now.

@mrrfv
Copy link
Owner

mrrfv commented Sep 17, 2023

Thanks! I’m feeling guilty for burdening you with so many requests, but there’s one more tiny thing. Could you please make it so that the download_lists Node script grabs both allowlists and blocklists when it’s run with no arguments? That’d be good for convenience.

@hlqviet
Copy link
Contributor Author

hlqviet commented Sep 17, 2023

Alright. Those requests just make the scripts work better so there's nothing guilty.

@mrrfv mrrfv merged commit dce6173 into mrrfv:main Sep 18, 2023
@mrrfv
Copy link
Owner

mrrfv commented Sep 18, 2023

Thanks for contributing so much :)

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