superatomic / homebrew-bundle-extensions

🗄 Command extensions for Homebrew that allow for easy modification of brew bundles.
https://gh.superatomic.dev/homebrew-bundle-extensions
BSD 2-Clause "Simplified" License
14 stars 2 forks source link

Add `--describe` arg to `brew add` #14

Closed ian-h-chamberlain closed 11 months ago

ian-h-chamberlain commented 11 months ago

Hi, I hope it's okay I didn't file an issue before implementing this! It was pretty simple to write, so I thought I'd just go ahead and open a PR.

This adds a --describe to insert a comment line with a description when brew adding a cask/formula. It also respects the existing HOMEBREW_BUNDLE_DUMP_DESCRIBE environment variable that has the same effect for brew bundle dump.

superatomic commented 11 months ago

Thank you for developing this feature! I would love to add this to brew add. Before this can be merged, there's a few things that need to be done.

Thanks again for this PR!

ian-h-chamberlain commented 11 months ago

I need your confirmation that you'd be willing to license your PR under both licenses.

Absolutely, this contribution can be licensed under both MIT and BSD-2-Clause. No concerns there, but thanks for checking.

I'd like to modify brew drop to remove the comments generated by --describe, so adding and removing a formula doesn't result in leftover comments persisting.

This makes sense and wasn't something I considered at first. I think I have a basic implementation working without too much hassle, so I'll go ahead and push it — but if you'd like to have something more robust in place then I'm happy to defer to your implementation as well.

Thanks for taking the time to look at this!

superatomic commented 11 months ago

Thank you! I have a few ideas for how to remove the description that I would like to try myself. A potential problem with your current implementation is that it breaks if the formula description itself is ever changed (or if the user modifies the comment, like you've described). My current idea that I'm planning to implement is for it to remove all comments that come directly before the line until it hits a line that contains either whitespace or a non-comment line (so dropwhile /^# / matches). My concern is that it could possibly remove too much, depending on how the user structures their Brewfile, but I can't think of that many situations where this would occur.

I'll get around to implementing this soon.

ian-h-chamberlain commented 11 months ago

Fair enough, it was a pretty naive attempt and I was guessing you had planned something a little more robust like you describe. Would you prefer I undo the changes I made to try and merge add --describe sooner, or wait for your planned implementation? If you'd rather merge it sooner I'm happy to update my branch, but no rush either way.

superatomic commented 11 months ago

I think it's probably best to implement both together.

superatomic commented 11 months ago

I've made some changes to the code and implemented the logic for brew drop. It looks ready to merge to me, but I'd love to hear your thoughts first before I do.