-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add dimmer-configure-magit #38
Conversation
Thank you! Do you mind updating the README and CHANGELOG as well? |
I just added a new commit with those changes. I didn't know what the bracketed number (e.g. [#37]) should be so I just left it out. |
Thank you, totally fine. If there had been a reported issue it would be that number. But it’s not worth opening a ticket just to close it again.
… On Mar 2, 2020, at 11:52 AM, tomkoelman ***@***.***> wrote:
Thank you! Do you mind updating the README and CHANGELOG as well?
I just added a new commit with those changes. I didn't know what the bracketed number (e.g. [#37 <#37>]) should be so I just left it out.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <#38?email_source=notifications&email_token=AASYZ5WN65DXWT6FMBN4MJ3RFQE6HA5CNFSM4K7SAXDKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENQWPPA#issuecomment-593586108>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASYZ5Q6HC6RTFPYK3OU2QLRFQE6HANCNFSM4K7SAXDA>.
|
Got it. I like the package, thanks for sharing.
… On 2 Mar 2020, at 20:59, Neil Okamoto ***@***.***> wrote:
Thank you, totally fine. If there had been a reported issue it would be that number. But it’s not worth opening a ticket just to close it again.
> On Mar 2, 2020, at 11:52 AM, tomkoelman ***@***.***> wrote:
>
> Thank you! Do you mind updating the README and CHANGELOG as well?
>
> I just added a new commit with those changes. I didn't know what the bracketed number (e.g. [#37 <#37>]) should be so I just left it out.
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub <#38?email_source=notifications&email_token=AASYZ5WN65DXWT6FMBN4MJ3RFQE6HA5CNFSM4K7SAXDKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENQWPPA#issuecomment-593586108>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASYZ5Q6HC6RTFPYK3OU2QLRFQE6HANCNFSM4K7SAXDA>.
>
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#38?email_source=notifications&email_token=AAJCZDAW3GDEPJ2WD54HLUTRFQFZ7A5CNFSM4K7SAXDKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENQXJAY#issuecomment-593589379>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJCZDF3LA2J6OESCII22YLRFQFZ7ANCNFSM4K7SAXDA>.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor changes noted in the comments. Thank you again for contributing!
- dimmer-configure-which-key was erroneously deleted - dimmer-configure-magit was mis-typed - convenvience functions weren't sorted alphabetically anymore
Thanks for the detailed feedback. I think I addressed every comment you made in the last three commits.
… On 2 Mar 2020, at 21:12, Neil Okamoto ***@***.***> wrote:
@gonewest818 requested changes on this pull request.
A few minor changes noted in the comments. Thank you again for contributing!
In README.md <#38 (comment)>:
> -* `dimmer-configure-which-key` is a convenience function for which-key
-users to ensure which-key popups are not dimmed.
It seems you accidentally deleted the documentation for dimmer-configure-which-key.
And actually, I'm trying to keep these dimmer-configure-xxx functions in alphabetical order, so can this move up to the appropriate place in the list? Same for the header comment in the source file.
In README.md <#38 (comment)>:
> +* `dimmer-configure-magitis a convenience function for magit users to
+ensure transients are not dimmed.
typo: you're missing the closing quote and a space is needed after dimmer-configure-magit
In dimmer.el <#38 (comment)>:
> @@ -264,6 +267,13 @@ uses a buffer name that fits this pattern."
(add-to-list
'dimmer-prevent-dimming-predicates #'which-key--popup-showing-p)))
+;;;###autoload
+(defun dimmer-configure-magit ()
+ "Convenience settings for magit users."
+ (with-no-warnings
Technically we don't need a with-no-warnings here. There are cases where we need to refer to predicate functions in other packages and the bytecompiler will complain that it doesn't recognize the symbol. But here the regexp doesn't depend on symbols in the package.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#38?email_source=notifications&email_token=AAJCZDAEDUQ5K6NEZF4RBLTRFQHK7A5CNFSM4K7SAXDKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCXTXCQA#pullrequestreview-367489344>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJCZDGPLHIUGYLLI3TKYGTRFQHK7ANCNFSM4K7SAXDA>.
|
Merged! Glad you like the package! |
I added
*transient*
todimmer-buffer-exclusion-regexps