Re: Washing GitHub emails to include inline patch?
Kyle Meyer writes: > Looking at what I wrote again, I'd change DONT-FETCH to FORCE-FETCH and > then do something like > > (when (or force-fetch > (not (magit-ref-exists-p local-ref))) > (magit-call-git "fetch" "origin")) > > where local-ref is bound to "refs/pull/origin/". That way, "git > fetch" is only called if the ref doesn't already exist locally or when a > prefix argument is given, which would be useful for forced updates. Oh, good call. >> -(magit-log (list (concat "master..refs/pull/origin/" pr) >> +(magit-log (list (concat "origin/master..refs/pull/origin/" pr) > > Anyway, it's nice to see that you've been able to modify this into > something that might be useful to you. Indeed, thanks again. The last piece of the puzzle is the origin/master branch isn't always the base branch it's merging into. I believe the proper way to do this is like so. First we add another set of refs to fetch in our .git/config: [remote "origin"] fetch = +refs/pull/*/merge:refs/merge/origin/* Now we can use this to get the base branch for the PR: refs/merge/origin/100^..refs/pull/origin/100 Which returns the proper set of commits. Cheers, -- https://jb55.com ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: Washing GitHub emails to include inline patch?
William Casarin writes: > This worked pretty well, I've modified it a bit to support a generic > github repo location which is parsed from the subject, Great, glad you were able to tweak that test function into something that works well with your setup. > and origin/master instead of master: Yep, that's better. > (defun km/notmuch-visit-pr-in-magit (&optional dont-fetch) > @@ -39,4 +41,4 @@ > ;; passing a more explicit refspec to the fetch call. > (unless dont-fetch >(magit-call-git "fetch" "origin")) Looking at what I wrote again, I'd change DONT-FETCH to FORCE-FETCH and then do something like (when (or force-fetch (not (magit-ref-exists-p local-ref))) (magit-call-git "fetch" "origin")) where local-ref is bound to "refs/pull/origin/". That way, "git fetch" is only called if the ref doesn't already exist locally or when a prefix argument is given, which would be useful for forced updates. > -(magit-log (list (concat "master..refs/pull/origin/" pr) > +(magit-log (list (concat "origin/master..refs/pull/origin/" pr) Anyway, it's nice to see that you've been able to modify this into something that might be useful to you. -- Kyle ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: cleartext indexing, round 3
Daniel Kahn Gillmor writes: > What follows is the third round of the latest revision of cleartext > indexing patches. pushed patches 1,2,3, and 5 to master. d ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v3 10/15] crypto: index encrypted parts when indexopts try_decrypt is set.
Daniel Kahn Gillmor writes:
> + if (status) {
> + _notmuch_database_log (notmuch, "Warning: setup failed for
> decrypting "
> +"during indexing. (%d)\n", status);
> + status = notmuch_message_add_property (message, "index-decryption",
> "failure");
> + if (status)
> + _notmuch_database_log (notmuch, "failed to add index-decryption
> "
> +"property (%d)\n", status);
> + return;
> + }
The second _notmuch_database_log will override the first
here. You can use _notmuch_database_log_append if you don't want to
clear the existing log (e.g. at least for the second _log
here).
> +const char *autoproperties[] = { "index-decryption" };
I'm always a bit nervous when I see the same string hard coded into two
different places. What about using some prefix naming scheme like
"index.auto.decryption" with the idea that all properties with the
prefix "index.auto." could be blown away. If we settle on a prefix based
naming scheme now, we could do the minor tweak to the properties API
later so that it's only a single call.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v3 04/15] crypto: move into libutil
Daniel Kahn Gillmor writes: > Hi Bremner-- > > Thanks for the review! > > On Thu 2017-10-12 07:54:33 -0300, David Bremner wrote: >> Daniel Kahn Gillmor writes: >> >>> This prepares us for using the crypto object in both the library and >>> the client. >> >> I think we could be more precise about names here, to help outsiders get >> up to speed on the code. libutil was renamed to libnotmuch_util, and >> "the library" should probably be "libnotmuch". > > Are you asking for improved comment text in the git commit, or for > something else? I'm happy to provide improved comment text, if that's > what you're asking for. > yes. >>> diff --git a/crypto.c b/util/crypto.c >>> similarity index 97% >>> rename from crypto.c >>> rename to util/crypto.c >>> index 4c1b7eec..39954ca0 100644 >>> --- a/crypto.c >>> +++ b/util/crypto.c >>> @@ -18,7 +18,11 @@ >>> * Authors: Jameson Rollins >>> */ >>> >>> -#include "notmuch-client.h" >>> +#include "crypto.h" >>> +#include "lib/notmuch-private.h" >>> + >> >> This seems like a kind of layering violation. What's in >> notmuch-private.h that needs to be exported to things outside of lib/ ? > > when building against gmime-3.0, this #include supplies: > > #define unused(x) x __attribute__ ((unused)) > > When building against gmime-2.6, it provides: > > #include > > > I could replace it with these two things explicitly (and i could put > them inside the GMIME_MAJOR_VERSION tests) if that would be preferable > to #including lib/notmuch-private.h. Any preference? I guess the most correct thing would be to have a header file in util/ which has the 'unused' definition in it and is included where it is needed. Or failing that to repeat the definition as we already do. d ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: Washing GitHub emails to include inline patch?
Kyle Meyer writes: > --8<---cut here---start->8--- > > (defun km/notmuch-github-pr-number () > "Return the PR number for this message." > (let (pr) > (with-current-notmuch-show-message > (goto-char (point-min)) > (if (re-search-forward "https://github\\.com/.*/pull/\\([0-9]+\\)" nil t) > (setq pr (match-string-no-properties 1)) >(user-error "Could not find PR number"))) > pr)) > > ;; This function could be anything that figures out the project based > ;; on the current notmuch message. Or, if you use projectile and > ;; don't mind getting queried each time, it could just read a project > ;; from `projectile-relevant-known-projects'. > (defun km/notmuch-repo-from-message () > "Return the repository that this message is associated with." > (let ((fname (notmuch-show-get-filename))) > (or (and fname > (string-match-p "magit-list" fname) > "~/src/emacs/magit") > (user-error "Could not determine repo" > > (defun km/notmuch-visit-pr-in-magit (&optional dont-fetch) > "Show the Magit log for this message's PR. > If DONT-FETCH is non-nil, do not fetch first." > (interactive "P") > (let* ((pr (km/notmuch-github-pr-number)) > (repo (km/notmuch-repo-from-message)) > (default-directory repo)) > ;; "origin" is hard-coded below, but it could of course be > ;; anything. You could also have an alist that maps repo -> > ;; remote. > ;; > ;; This assumes that you've added > ;; > ;;fetch = +refs/pull/*/head:refs/pull/origin/* > ;; > ;; to origin's in ".git/config". You could drop that assumption > ;; passing a more explicit refspec to the fetch call. > (unless dont-fetch > (magit-call-git "fetch" "origin")) > (magit-log (list (concat "master..refs/pull/origin/" pr) > --8<---cut here---end--->8--- This worked pretty well, I've modified it a bit to support a generic github repo location which is parsed from the subject, and origin/master instead of master: --- kyle1.el 2017-10-12 12:22:01.977663605 -0700 +++ kyle2.el 2017-10-12 12:24:49.644673189 -0700 @@ -14,10 +14,12 @@ ;; from `projectile-relevant-known-projects'. (defun km/notmuch-repo-from-message () "Return the repository that this message is associated with." - (let ((fname (notmuch-show-get-filename))) -(or (and fname - (string-match-p "magit-list" fname) - "~/src/emacs/magit") + (let ((subject (notmuch-show-get-subject)) +(repo)) +(or (and subject + (or (and (string-match "\\([^\]]+\\)" subject 1) + (setq repo (match-string-no-properties 1 subject)) + (concat "~/dev/github/" repo (user-error "Could not determine repo" (defun km/notmuch-visit-pr-in-magit (&optional dont-fetch) @@ -39,4 +41,4 @@ ;; passing a more explicit refspec to the fetch call. (unless dont-fetch (magit-call-git "fetch" "origin")) -(magit-log (list (concat "master..refs/pull/origin/" pr) +(magit-log (list (concat "origin/master..refs/pull/origin/" pr) Cheers, William -- https://jb55.com ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v4 08/15] index: implement notmuch_indexopts_t with try_decrypt
On Thu 2017-10-12 08:18:15 -0300, David Bremner wrote: > Daniel Kahn Gillmor writes: > >> >> +if (!indexopts) >> +indexopts = def_indexopts = notmuch_database_get_default_indexopts >> (notmuch); >> + > > I think I'd like parens here > > indexopts = (def_indexopts = notmuch_database_get_default_indexopts > (notmuch)); > > or even split into two assignments. Others might disagree though. I've gone with the simpler variant of splitting into two assignments. My updated branch is still cleartext-index on https://gitlab.com/dkg/notmuch, with commit ID 77e8c5079e854d1934e47637c15f06bbc14a4819. Shall i push this to the mailing list as v5 of this series, or are there more reviews coming that i should try to integrate? --dkg signature.asc Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v3 04/15] crypto: move into libutil
Hi Bremner-- Thanks for the review! On Thu 2017-10-12 07:54:33 -0300, David Bremner wrote: > Daniel Kahn Gillmor writes: > >> This prepares us for using the crypto object in both the library and >> the client. > > I think we could be more precise about names here, to help outsiders get > up to speed on the code. libutil was renamed to libnotmuch_util, and > "the library" should probably be "libnotmuch". Are you asking for improved comment text in the git commit, or for something else? I'm happy to provide improved comment text, if that's what you're asking for. >> diff --git a/crypto.c b/util/crypto.c >> similarity index 97% >> rename from crypto.c >> rename to util/crypto.c >> index 4c1b7eec..39954ca0 100644 >> --- a/crypto.c >> +++ b/util/crypto.c >> @@ -18,7 +18,11 @@ >> * Authors: Jameson Rollins >> */ >> >> -#include "notmuch-client.h" >> +#include "crypto.h" >> +#include "lib/notmuch-private.h" >> + > > This seems like a kind of layering violation. What's in > notmuch-private.h that needs to be exported to things outside of lib/ ? when building against gmime-3.0, this #include supplies: #define unused(x) x __attribute__ ((unused)) When building against gmime-2.6, it provides: #include I could replace it with these two things explicitly (and i could put them inside the GMIME_MAJOR_VERSION tests) if that would be preferable to #including lib/notmuch-private.h. Any preference? --dkg signature.asc Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v4 08/15] index: implement notmuch_indexopts_t with try_decrypt
Daniel Kahn Gillmor writes: > > + if (!indexopts) > + indexopts = def_indexopts = notmuch_database_get_default_indexopts > (notmuch); > + I think I'd like parens here indexopts = (def_indexopts = notmuch_database_get_default_indexopts (notmuch)); or even split into two assignments. Others might disagree though. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v3 04/15] crypto: move into libutil
Daniel Kahn Gillmor writes: > This prepares us for using the crypto object in both the library and > the client. I think we could be more precise about names here, to help outsiders get up to speed on the code. libutil was renamed to libnotmuch_util, and "the library" should probably be "libnotmuch". > diff --git a/crypto.c b/util/crypto.c > similarity index 97% > rename from crypto.c > rename to util/crypto.c > index 4c1b7eec..39954ca0 100644 > --- a/crypto.c > +++ b/util/crypto.c > @@ -18,7 +18,11 @@ > * Authors: Jameson Rollins > */ > > -#include "notmuch-client.h" > +#include "crypto.h" > +#include "lib/notmuch-private.h" > + This seems like a kind of layering violation. What's in notmuch-private.h that needs to be exported to things outside of lib/ ? ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
