Rewrite the Linux part #43
Conversation
Create a template of background-manager based on previous template, but with 3 mehods : get, set, available Adapt the function set get and setAvailables
|
@deather Sorry about the long delay. I've had too many PRs to review. This is looking great! It's a much better and more maintainable way to structure it. |
Good idea! I like it. |
Moving promisify function in util.js file. Renaming function existsCommand into commandExists Removing the usage of exec Using Map instead of Object for storing the wallpaper vote Using String template instead of String concatenation Renaming function available into isAvailable
| const {stdout} = await execFile('dconf', ['read', '/org/mate/desktop/background/picture-filename']); | ||
|
|
||
| return stdout.trim().slice(1, -1); | ||
| } catch (_) {} |
There was a problem hiding this comment.
Yes, the responsibility to ignore the errors should be in the consumer, not the get function.
You can do something like this to ignore errors:
await Promise.all(promises.map(promise => promie.catch(() => {})));| const {stdout} = await execFile('dconf', ['read', '/org/mate/desktop/background/picture-filename']); | ||
|
|
||
| return stdout.trim().slice(1, -1); | ||
| } catch (_) {} |
There was a problem hiding this comment.
Actually, would be better to just change line 31 to const imagePath = await app.get().catch(() => {});
Find and replace to use arrow functions. Use of Object.values() instead of Object.entries() Removing all useless try catch to catch errors in the global get. Refactoring grep function into hasLine Fixing bug between dconf.js and mate.js. When we set picture-filename with dconf it is equivalent to the usage of gsettings. So now we just set the property with the filename not the URI.
|
This is looking great now. Thank you so much for improving this, @deather 🙌 |
|
@deather Would you be interested in joining the project as a maintainer? |
|
@sindresorhus It will be cool, you can rely on me for this project 😄 |
This PR is in response of issue #42
I purpose you a new file organization for manage the multiple background manager on Linux.
I'm using nitrogen so for me it is working. It would be cool if some people could test before merge.
What is it changed ?
setAvailableAppsiterate over all manager and test if they are available, if it is they are storedsetmethod just call thesetmethod of each available background managersgetmethod is more difficult to write, due to multiple managers. I use a strategy consisting of count the apparition of a file, and the most used will be return.The final point is personnal, I think it is the more discutable. I'm ok to debate on this point.
I hope this work can help.
Thanks for reading.