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

Expose 'DHT bootstrap nodes' setting #19594

Merged
merged 1 commit into from Sep 14, 2023
Merged

Conversation

Chocobo1
Copy link
Member

This allows user to select DHT bootstrap nodes. Or even use their own bootstrap nodes.

@Chocobo1 Chocobo1 added WebUI WebUI-related issues/changes GUI GUI-related issues/changes labels Sep 11, 2023
@Chocobo1 Chocobo1 added this to the 5.0 milestone Sep 11, 2023
@thalieht
Copy link
Contributor

This setting requires a way to reset it to default IMO. If it's accidentally deleted, users would have to go search in the source code (unlikely) or come ask about it here.

@Chocobo1
Copy link
Member Author

Chocobo1 commented Sep 12, 2023

This setting requires a way to reset it to default IMO. If it's accidentally deleted, users would have to go search in the source code (unlikely) or come ask about it here.

What about users clicking on the (?) link and they can see a value from libtorrent doc, wouldn't that be sufficient?

ps. after all, it is advanced settings, we can't help if users messes with it and shoot their feet...

@glassez
Copy link
Member

glassez commented Sep 12, 2023

This setting requires a way to reset it to default IMO. If it's accidentally deleted, users would have to go search in the source code (unlikely) or come ask about it here.

Empty string could load default nodes.

@glassez
Copy link
Member

glassez commented Sep 12, 2023

ps. after all, it is advanced settings, we can't help if users messes with it and shoot their feet...

I think it's not worth neglecting some friendliness after all, if it's as trivial as what I suggested above.

@Chocobo1
Copy link
Member Author

Chocobo1 commented Sep 12, 2023

I think it's not worth neglecting some friendliness after all, if it's as trivial as what I suggested above.

How about I change the setting semantics to: if the lineedit widget is empty use the default nodes, otherwise use the user provided value? However, the default node URLs will not be visible to users...

@glassez
Copy link
Member

glassez commented Sep 12, 2023

How about I change the setting semantics to: if the lineedit widget is empty use the default nodes, otherwise use the user provided value?

That's what I was suggested.
BTW, lineedit can have a placeholder displaying these default values, or just [Use default bootstrap nodes].

@Chocobo1
Copy link
Member Author

Chocobo1 commented Sep 12, 2023

BTW, lineedit can have a placeholder displaying these default values, or just [Use default bootstrap nodes].

The default nodes are too long to be shown entirely when using placeholder text. And I consider it important to show the full list to the user.
After some experiments I decided that if the user had (entirely) cleared the list then it will repopulate/reset to default list automatically.

This allows user to select DHT bootstrap nodes. Or even use their own bootstrap nodes.
@Chocobo1 Chocobo1 merged commit dcba9ed into qbittorrent:master Sep 14, 2023
13 checks passed
@Chocobo1 Chocobo1 deleted the dht branch September 14, 2023 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GUI GUI-related issues/changes WebUI WebUI-related issues/changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants