-
-
Notifications
You must be signed in to change notification settings - Fork 564
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
Manage adlists #252
Manage adlists #252
Conversation
Could we add a text field on the bottom for custom lists? Then they can add them there. Also, how will we handle if the lists are changed in the repo and their |
Sure, I can add a field where a user can add lists. I'd store them in an extra file, so the original |
I think it's more of a deeper problem. Since |
Currently I do the following: Edit This might be some level of inconvenience connected to each update, but we can be sure that all users have a working situation (that we can control) after each update. Re-enabling the extra lists is done in seconds. |
Conflicts: php/savesettings.php
…e list maintained by us will be reset on every update
…fficiency + speed)
… time (one URL per line)
Updated screenshot in PR description |
Gravity actually already overrides the default list on every run. ;) https://github.com/pi-hole/pi-hole/blob/master/gravity.sh#L335 |
That sounds like it should work well. Just will have to try and re-enable/disable the lists that are in the default list based on the user's prior settings, if possible. |
Does this require any Core changes? |
php/savesettings.php
Outdated
{ | ||
while (($line = fgets($handle)) !== false) | ||
{ | ||
if(substr($line, 0, 2) === "#h") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this check for #http
instead? There might be a comment that starts like #here's some list...
php/savesettings.php
Outdated
function readAdlists(&$list, $listname) | ||
{ | ||
// Reset list | ||
$list = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why take in a list and just rewrite it? Could this return a list instead?
$adlistsuser = readAdlists("user");
@Mcat12 These are the core changes: |
WIP since we might change the format of the adlist to CSV |
i oticed this thread a few days ago and am wondering if the formatting of my personal adlist could be useful in it
|
Why should the user want to delete them when he can disable them? |
is there a way to label the lists using a more descriptive title if is is present in the adlists.list file ? |
@DL6ER some people might just want to have a clean slate, and not use any of the defaults. Why have them all in the list disabled if they're never going to use them? @technicalpyro possibly, for now I think it's nice just to get the functionality out there and work on the polish separately! |
Changes proposed in this pull request:
This section is loaded in collapsed form since it needs quite some space of the Settings page...
@pi-hole/dashboard