Skip to content
This repository was archived by the owner on Oct 15, 2025. It is now read-only.

Fix syncthing plugin #223

Closed
ngencokamin wants to merge 3 commits into from
Closed

Fix syncthing plugin #223

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ed8737a
Fix syncthing
ngencokamin Jul 3, 2025
2567b33
Revert settings label change, tooltip
ngencokamin Jul 8, 2025
e872882
Remove unused code for debugging that I accidentally left in
ngencokamin Jul 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
151 changes: 84 additions & 67 deletions syncthing/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,16 @@

class Plugin(PluginInstance, GlobalQueryHandler):

config_key = 'syncthing_api_key'
config_key = 'api_key'
Copy link
Author

@ngencokamin ngencokamin Jul 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to make the config key consistent with references throughout the rest of the script

Copy link
Contributor

@Sharsie Sharsie Jul 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the code, it seems that self.st is only defined when api key is set. So for anyone using this plugin and having that config present (maybe from a past version), this would be a breaking change. If I'm understanding this correctly, the plugin is not working properly for someone enabling it for the first time.
I would recommend keeping the old config key name not to force people into having to input it again.


def __init__(self):
PluginInstance.__init__(self)
GlobalQueryHandler.__init__(self)

self.iconUrls = ["xdg:syncthing", f"file:{Path(__file__).parent}/syncthing.svg"]
self._api_key = self.readConfig(self.config_key, str)
if self._api_key is None:
self._api_key = ''
Comment on lines +33 to +34
Copy link
Author

@ngencokamin ngencokamin Jul 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes the widget not appearing with error 12:26:44 [crit:albert.python] Unable to cast Python instance of type <class 'NoneType'> to C++ type 'QString'

Basically just initializes api_key to an empty string if an api key isn't found in the config. Otherwise it breaks because of it being None/null

Copy link
Contributor

@Sharsie Sharsie Jul 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be shortened to

self._api_key = self.readConfig(self.config_key, str) or ""

if self._api_key:
self.st = Syncthing(self._api_key)

Expand All @@ -39,6 +41,7 @@ def defaultTrigger(self):
@property
def api_key(self) -> str:
return self._api_key
# return '1234'
Copy link
Author

@ngencokamin ngencokamin Jul 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't mean to leave this in and will make a commit removing it

Copy link
Author

@ngencokamin ngencokamin Jul 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, this is fixed now


@api_key.setter
def api_key(self, value: str):
Expand All @@ -65,76 +68,90 @@ def configWidget(self):
def handleGlobalQuery(self, query):

results = []

if self.st:

config = self.st.system.config()

devices = dict()
for d in config['devices']:
if not d['name']:
d['name'] = d['deviceID']
d['_shared_folders'] = {}
devices[d['deviceID']] = d

folders = dict()
for f in config['folders']:
if not f['label']:
f['label'] = f['id']
for d in f['devices']:
devices[d['deviceID']]['_shared_folders'][f['id']] = f
folders[f['id']] = f

matcher = Matcher(query.string)

# create device items
for device_id, d in devices.items():
device_name = d['name']

if match := matcher.match(device_name):
device_folders = ", ".join([f['label'] for f in d['_shared_folders'].values()])

actions = []
if d['paused']:
actions.append(
Action("resume", "Resume synchronization",
lambda did=device_id: self.st.system.resume(did))
)
else:
actions.append(
Action("pause", "Pause synchronization",
lambda did=device_id: self.st.system.pause(did))
try:
if self.st:
config = self.st.system.config()

devices = dict()
for d in config['devices']:
if not d['name']:
d['name'] = d['deviceID']
d['_shared_folders'] = {}
devices[d['deviceID']] = d

folders = dict()
for f in config['folders']:
if not f['label']:
f['label'] = f['id']
for d in f['devices']:
devices[d['deviceID']]['_shared_folders'][f['id']] = f
folders[f['id']] = f

matcher = Matcher(query.string)

# create device items
for device_id, d in devices.items():
device_name = d['name']

if match := matcher.match(device_name):
device_folders = ", ".join([f['label'] for f in d['_shared_folders'].values()])

actions = []
if d['paused']:
actions.append(
Action("resume", "Resume synchronization",
lambda did=device_id: self.st.system.resume(did))
)
else:
actions.append(
Action("pause", "Pause synchronization",
lambda did=device_id: self.st.system.pause(did))
)

item = StandardItem(
id=device_id,
text=f"{device_name}",
subtext=f"{'Paused ' if d['paused'] else ''}Syncthing device. "
f"Shared: {device_folders if device_folders else 'Nothing'}.",
iconUrls=self.iconUrls,
actions=actions
)

results.append(RankItem(item, match))

# create folder items
for folder_id, f in folders.items():
folder_name = f['label']
if match := matcher.match(folder_name):
folders_devices = ", ".join([devices[d['deviceID']]['name'] for d in f['devices']])
item = StandardItem(
id=folder_id,
text=folder_name,
subtext=f"Syncthing folder {f['path']}. "
f"Shared with {folders_devices if folders_devices else 'nobody'}.",
iconUrls=self.iconUrls,
actions=[
Action("scan", "Scan the folder",
lambda fid=folder_id: self.st.database.scan(fid)),
Action("open", "Open this folder in file browser",
lambda p=f['path']: openFile(p))
]
)
results.append(RankItem(item, match))
Comment on lines +70 to +139
Copy link
Author

@ngencokamin ngencokamin Jul 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bit try is meant to catch it when syncthing fails to initialize due to missing or invalid api key. Otherwise it spits errors with every character entered into the launcher. Plus, having it blank with no explanation outside of the console is not the most friendly design.

Copy link
Author

@ngencokamin ngencokamin Jul 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I maybe could do the try except only around the bit that checks if st is initialized/valid? So that it's a little less general

Copy link
Contributor

@Sharsie Sharsie Jul 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest a few points to refactor this and make it more robust

First: I would look into api_key setter. It only sets self.st if the API key changes, but that includes if api key is empty. I'm not that familiar with Syncthing to know if this is allowed, but I assume you need an API key.
So instead of creating self.st every time, I would set it to None if api key is empty.

Second: You wouldn't need exception handling for empty API key. Instead, the first thing in handleGlobalQuery could be a check to self.st (that is now properly set due to the First point) and return the StandardItem with information about missing API key, something along the lines of

if not self.st:
  return [StandardItem(...)]

Third: You should keep the try block for any call to syncthing, do not wrap it only for the initial call. It could go down between the loop iterations and you need to catch those errors. However, printing out "Invalid API key" may not be appropriate, as Syncthing could just be down/not available for any reason. To make it more readable, exporting this logic into another function and then calling that function in a try except block is also an option

Copy link
Author

@ngencokamin ngencokamin Jul 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm crazy busy for the next few weeks, but I will mark this as draft for now and refactor when I am able. I'll let y'all know when it's ready for re-review

except:
if self._api_key == '':
item = StandardItem(
id=device_id,
text=f"{device_name}",
subtext=f"{'Paused ' if d['paused'] else ''}Syncthing device. "
f"Shared: {device_folders if device_folders else 'Nothing'}.",
iconUrls=self.iconUrls,
actions=actions
id="no_key",
text="Please enter your Syncthing API key in settings",
subtext="You can find your api key on your Syncthing web dashboard.",
iconUrls=self.iconUrls
)

results.append(RankItem(item, match))

# create folder items
for folder_id, f in folders.items():
folder_name = f['label']
if match := matcher.match(folder_name):
folders_devices = ", ".join([devices[d['deviceID']]['name'] for d in f['devices']])
else:
item = StandardItem(
id=folder_id,
text=folder_name,
subtext=f"Syncthing folder {f['path']}. "
f"Shared with {folders_devices if folders_devices else 'nobody'}.",
iconUrls=self.iconUrls,
actions=[
Action("scan", "Scan the folder",
lambda fid=folder_id: self.st.database.scan(fid)),
Action("open", "Open this folder in file browser",
lambda p=f['path']: openFile(p))
]
id="invalid_key",
text='Invalid API Key',
subtext=f"API Key {self._api_key} is invalid. Please try entering your API key again in settings.",
iconUrls=self.iconUrls
)
results.append(RankItem(item, match))

results.append(RankItem(item, 0))
Comment on lines +141 to +155
Copy link
Author

@ngencokamin ngencokamin Jul 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So in theory, this except should only fire if the api key is invalid or missing. But I can see how we might want the logic to be a little less broad. Basically rn it just checks if the key is blank, at which point it reports missing, else it reports invalid.

return results