Generalized most of the pairs of -pr- and -commit- prefixed methods #63
Conversation
that intended to work on the pr and commit document respectively into common methods. The type of the document is deduced from the document descriptor alist. Fixed read-only buffer case that can turn in due to automatic diff-mode on .diff extension. Extended `github-review-save-diff` with `github-review-save-commit-id-in-buffer` branch to controls whether to add the commit and its parent id info to the buffer as persistent buffer-locals.
| (when (not (null ids)) | ||
| (set-variable 'gh-commit-id (first ids) t) | ||
| (set-variable 'gh-first-parent-id (second ids) t) | ||
| (insert |
There was a problem hiding this comment.
What is the rationale for setting them here instead of making them buffer local? Is it because you want this to persist across session? The downside is that the user will be typically prompted to accept those on a per buffer basis which increases friction to start any review
There was a problem hiding this comment.
Is it because you want this to persist across session?
Right.
The prompting downside is just one time as the var:s can be added to
safe-local-variable-values at the first file opening.
Alternatively the variables could be placed elsewhere as plain ~"comments"
to be processed by who needs them (I obviously do).
You may have noticed already that .diff extension does not let these file local var:s be initialized at the file re-opening. The reason is the value of inhibit-local-variables-regexps which holds .diff. I am thinking to customize the github patch file extension and maybe provide the default to that like .gh-diff.
There was a problem hiding this comment.
To follow up with the user-customizable file extension, it more makes sense
with a hook at the end of (github-review-save-diff). That is my preference block
+ (when github-review-save-commit-id-in-buffer
+ (when (not (null ids))
+ (set-variable 'gh-commit-id (first ids) t)
+ (set-variable 'gh-first-parent-id (second ids) t)
would go into my personal hook.
|
Thanks for your work, this is a nice simplification! I left one comment inline for you to take a look at. |
Thank you, Laurent! When we'll make it through, I already have another feature request in progress, to share with you hopefully sooner :-) |
|
@charignon, howdy. After a break caused by my load at workplace, I am resuming with this request. I am trying to rebase on the up-to-date Cheers, Andrei |
|
@charignon, resuming on my Apr 5 target, having time to understand how to make it the right way (git merge is not). Hope that is not interleaves with your generous merging plan (in which case just tell me to step away). |
that intended to work on the pr and commit document respectively into
common methods. The type of the document is deduced from the document
descriptor alist.
Fixed read-only buffer case that can turn in due to automatic diff-mode
on .diff extension.
Extended
github-review-save-diffwithgithub-review-save-commit-id-in-bufferbranch to controls whether toadd the commit and its parent id info to the buffer as persistent
buffer-locals.