kosync.koplugin: Add custom hostname option for kosync plugin#14172
Conversation
|
|
||
| function KOSync:setHostname(hostname) | ||
| logger.dbg("KOSync: Setting custom hostname to:", hostname) | ||
| self.settings.kosync_hostname = hostname ~= "" and hostname or nil |
There was a problem hiding this comment.
To delete the custom hostname you need to allow blank input:
koreader/frontend/ui/widget/container/inputcontainer.lua
Lines 381 to 386 in 9017bcc
There was a problem hiding this comment.
@rspilk Could you please resolve this? If I understand correctly it only needs to be made simpler like this?
| self.settings.kosync_hostname = hostname ~= "" and hostname or nil | |
| self.settings.kosync_hostname = hostname |
There was a problem hiding this comment.
This this cause hostname to be reevaluated as Device.model though? I thought it had to be nil in order for that to work and leaving it empty would be ""?
I looked at adding allow_blank_input = true into the tap input function but I couldn't seem to make that work in my testing. I will try a bit on it this weekend when I have a bit of time to put into getting my environment set up again
|
I made an additional change to the main.lua. The menu function relies on input not being blank for the other elements. I thought making this change to the one section of the menu was a better option than globally affecting the menu allowing blank entries but I am open to hearing what you think. Now making it empty reverts back to Device.model or syncs with whatever you enter in the field. I am also not sure how a pull request works now that the master branch has moved on but i dont believe the file has been modified otherwise |
|
| return { | ||
| callback = function() | ||
| local dialog | ||
| dialog = MultiInputDialog:new{ |
There was a problem hiding this comment.
MultiInputDialog with one field is InputDialog.
| title = _("Hostname for sync"), | ||
| fields = { | ||
| { | ||
| text = self.settings.kosync_hostname or "", |
There was a problem hiding this comment.
No need for or, the input accepts nil.
| self.settings.custom_server = server ~= "" and server or nil | ||
| end | ||
|
|
||
| function KOSync:setHostname(hostname) |
There was a problem hiding this comment.
Just 2 lines, the method isn't called anywhere else, can de done right in the callback.
There was a problem hiding this comment.
I see, you follow the style of the module, okay.
There was a problem hiding this comment.
I see, you follow the style of the module, okay.
Is that the case? There's still a difference between having a function with only a couple of lines and having a function that's only called from a single spot. But yeah, getters and setters for everything even when nothing's happening yet is this Java/OOP pattern I'm not really into either.
|
I made a change to the plugin to use the InputDialog function instead of MultiInput. I also followed the advice to remove the function and perform directly in the callback, its cleaner but it is slightly different than some of the other stylings. I don't care either way so I could revert it. Also noticed that InputDialog uses the name "input_hint" for its hinting and MultiInputDialog uses "hint" so i modified to accommodate that. Thats probably a bigger UI change that would have to be made to fix that globally. |
|
I'm not sure that I'm happy with the latest massive style editing. |
|
Me neither. ;-) |
9360e2b to
edefb07
Compare
|
Fixed that Sorry about the accidental reformatting. The diff should now only contain the intended logic changes. |
| end, | ||
| }, | ||
| { | ||
| text = _("Device Hostname"), |
…Dialog. Additionally use inline callback rather than a whole placeholder function.
|
The build failed after modifying that string? |
|
I'm not immediately sure what you're referring to. I only see green checkmarks on pretty much every commit? |
Adds a custom hostname option for the kosync plugin. This feature allows a user-defined device name to be set in settings.reader.lua, which is then used instead of the default Device.model value for progress updates sent to the kosync server. This provides a more explicit way to identify a device, which is useful when syncing across multiple readers.
The feature is backward compatible; if no custom hostname is set, it defaults to the existing Device.model behavior. This has been tested and confirmed to be working with koreader/kosync:latest (VERSION=1.0.1.3)
This change is