Add --describe arg to brew add

[ian-h-chamberlain]

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]

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.

• This project is actually duel-licensed under the BSD-2-Clause License and the MIT License as part of the efforts to merge this tap with homebrew/homebrew-bundle as a core feature (see brew bundle add && brew bundle remove Homebrew/homebrew-bundle#818). I had forgotten to update the licensing information in this repository to reflect that, which I have now rectified in Add MIT License #15. Of course, since you submitted this PR before I clarified the change, I need your confirmation that you'd be willing to license your PR under both licenses.
• 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. I am willing to implement this myself, so no action is required on your part, but if you want to try to implement it yourself, you are more than welcome to do so. Changing brew drop in this way will probably require a significant rework of the current code to be able to remove lines that have already been checked.

Thanks again for this PR!

[ian-h-chamberlain]

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]

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]

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]

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

[superatomic]

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.

[ian-h-chamberlain]

Just curious, why bother with this ?! match here? To avoid capturing places where someone commented out a line like # brew "foo" ?

[superatomic]

Yes, that's the goal. It's a pretty ugly hack but I think it'll address the most common cases where a comment would otherwise incorrectly be removed.

[ian-h-chamberlain]

LGTM! Not sure about exactly what cases you want to cover with drop but the logic as written seems like it would cover what was added by --describe.