ledger / ledger-mode

Emacs Lisp files for interacting with the C++Ledger accounting system
GNU General Public License v2.0
381 stars 76 forks source link

bind M-; to comment/uncomment an entire transaction #315

Open presto8 opened 3 years ago

presto8 commented 3 years ago

Thank you for writing ledger-mode, it has truly revolutionized my use of ledger!

One feature I would find nice is the ability to comment, uncomment, or toggle an entire transaction. Sometimes it is desirable to comment out a problematic transaction that is causing problems (for example, balance assertions). It would be nice to be able to simply hit M-; (or other hotkey) which would comment or uncomment the current transaction under the point.

I am more than happy to try adding this feature to the source and submitting a pull request, but wanted to check first if it is something that would be desirable.

Thank you!

enderw88 commented 3 years ago

Sounds good to me. I would use it. Sorry I didn’t think of it

On Fri, Oct 1, 2021 at 08:07 Preston Hunt @.***> wrote:

Thank you for writing ledger-mode, it has truly revolutionized my use of ledger!

One feature I would find nice is the ability to comment, uncomment, or toggle an entire transaction. Sometimes it is desirable to comment out a problematic transaction that is causing problems (for example, balance assertions). It would be nice to be able to simply hit M-; (or other hotkey) which would comment or uncomment the current transaction under the point.

I am more than happy to try adding this feature to the source and submitting a pull request, but wanted to check first if it is something that would be desirable.

Thank you!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ledger/ledger-mode/issues/315, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGWWLESNYFYFOIDKYJ5KVDUEXFC5ANCNFSM5FE424DA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- Craig, Corona De Tucson, AZ [image: missile_flyout] enderw88.wordpress.com

purcell commented 3 years ago

Users might reasonably have different expectations of what the common keybinding M-; might operate on, e.g. current line vs. current transaction. I'd suggest it should be easy enough to write a command that will call comment-region/uncomment-region on the transaction at point, but that it should probably not be bound to any particular key by default.

presto8 commented 3 years ago

Thanks for the feedback so far. Yes, I see the point about redefining key bindings which may introduce surprises for users used to the current behavior. In looking at the code, it was extremely easy to duplicate the approach used to apply the font highlighting for the current transaction. I'll submit a pull request soon. Thanks!