m-cli
m-cli copied to clipboard
rgcr
Add many, many plugins and settings
This is a fairly massive PR, which isn't exactly good form, but:
- I put in a ton of commits so it should be fairly straightforward to follow
- Many of the changes required changes on every plugin, which would have been extremely difficult to PR
- I wanted to get this done
My main reason for wanting to get these into m-cli is simply because I wanted them out in the community so that fixes could be done in one place rather than in 90,000 dotfiles strewn across Github.
These changes focus on defaults style commands. I've spend dozens (maybe hundreds) of hours collecting these over the years such that I can now boot up a fresh OS install, run a script, reboot and it's set up exactly as I'd like. Hopefully now that these are all out in the community, we can stop doing these in our own silos and instead combine our resources.
There are things about this PR that could be improved (such as the way the "current" value of some of these commands are output) however (not counting the untold hours I spent actually finding which plist files changed when a setting changed), I've already spend three full days converting my old scripts to m-cli and I can't devote any more resources to this.
I definitely think it's mergable. I cause no (that I know of) breaking changes so it should be a minor Semver release.
Wow. Loads of good stuff here. Almost impossible to safely review though..
@thegranddesign this is amazing!, can you help to create the proper completions for those plugins? Let me test it before to do the merge.
@rgcr thanks. :) Unfortunately I've spent way too much time on this already. I ran most of the commands against a fresh OS install of Sierra and it was about a 90% success rate. Which may mean that some of the settings need version checks (100% of them worked in 10.10).
I wish I could contribute further, but someone else is going to need to run it over the finish line.
Additionally, some review will be needed to decide which of these commands need which process to be killed. I run this as a one time thing after a fresh OS install and so all I do is restart. I've never taken the time to try to make it work after each individual command is run.
I'm going to work on this, but I think we should keep it as simple as we can, I mean every 'plugin' should be independent of each other, so I will implement all plugins of this PR but in a simplest way, I think there are a lot of helper functions that are not necessary.
@rgcr I could help you with separating those into PRs if you can provide a base those plugins should use, e.g. necessary helpers etc.
@shpoont, That would be awesome and very helpful, I would like to remove just the dependency of lib/defaults.sh and lib/converters.sh on all plugins, add the new ones and add the new functionalities in the old ones.
@rgcr sounds good, let me know when its ready? I'll then pick one of the plugins and make a test PR
@rgcr I really don't understand why you would want to remove the "dependency" of those files. You have (in the current codebase) many multiples of places where those exact things are happening. Which means you have many places where you have the possibility of a bug. In fact, when I went through and refactored, I found many bugs where code had been copy/pasted wrong or a bug had been fixed in one place, but not fixed in the other half dozen places where that "same" code was being used.
Using those lib files, if there's a bug, you fix it in one place and it fixes it everywhere. Having those helper functions is just good DRY methodology.
Not to mention, when you remove all the parsing complexity, it makes the actual plugins themselves a lot easier to read and understand.
Just as an example:
Before
https://github.com/rgcr/m-cli/blob/ba3af98fb8be91595f8e9782a4a333c75e7d770b/plugins/finder#L27-L57
After
hidden_files(){
value="$(_mcli_defaults_yes_no_to_boolean "com.apple.finder" \
"AppleShowAllFiles" \
"$1")"
echo "${command} ${subcommand}: ${value}"
}
@rgcr This is really awesome. What kind of work would you like to see on it to get it into a merge-able state?
I'd like to pitch in with getting this stuff in. I agree with @thegranddesign that this would remove a lot of my need to refer to various dotfiles. Just need a little guidance on direction from @rgcr.
@bensleveritt go ahead, at this moment I don't have enough time work on it, just resolve the conflicts and be sure everything works as expected even the completions.
Looking at all this, I'm tempted to break this into lots of little PRs.
If you're happy to accept the utilities lib as the first PR @rgcr, it'll enable me to go through each plugin and supply any changes that @thegranddesign did. This'll make reviewing a lot easier, and technically safer.
Are you happy with that approach?
My biggest reservation is losing credit for @thegranddesign for all the work they did.
@bensleveritt, Yes I think that is the best approach. And don't worry I'll update the README to give credit to him and you. 😉
@bensleveritt trying to break this up into multiple PRs is going to be a nightmare. There a ton of extraction to create the helper functions. The commits are already in "story order". I don't see how creating a bunch of PRs makes it any more manageable. Just look at the commits in order and approve them.
@thegranddesign Haa.. You're not wrong about it being a lot of work, but the fact this PR has been stuck for over two years is a good indication that it's asking too much of folks to review. You yourself recognize it's bad form, and yet still expect others to do the work.
Since this PR, there have been other submissions for the same or similar functionality, and there are conceptual conflicts, as well as the obvious technical conflicts.
My intention is to reduce all of your changes into plugin-specific PRs, so if any parts aren't accepted, the others still can be. It's important that discussion happens in the context of the changes.
To be clear, the work you've committed is laudable, I'm just breaking it into more acceptable chunks.
@bensleveritt To be clear, I don't expect anyone else to do the work, I simply said I had already spent multiple weeks on the code as well as writing commits to make it easy to review. And that I wasn't going to take more time to split it up. This could have been merged as it was with no additional work on anyone's part (other than review) 2.5 years ago. This PR caused almost zero breaking API changes at the time. It could have easily been merged and then tweaked post-merge.
For my part I put it up here to give back, and it's been disappointing not seeing it get merged, but my fork works great for everything I use it for and that's good enough for me, and anyone else who wants to use that instead.
I definitely wish you good luck.
@thegranddesign as you wrote, that was a lot of work, unfortunately as you I didn't have too much time to review the PR at that time , I appreciate your work and the time you spent on this.
@bensleveritt thank you for being doing the hard work here.
Once all changes are reviewed and merged I'll close this PR
@rgcr Any update on this?
Ah, it's been a while since I've used m. Last I recall working on was working out which modules could be imported which don't already exist. Then existing modules could be checked to see if there was anything to port across.
To this end, I've got a board of progress started https://github.com/bensleveritt/m-cli/projects/1 which I'll update as I move the plugins across.