Morgan Smith <[email protected]> writes: > I'm sure Ihor wouldn't mind me taking over this review. > > Thank you very much for taking a look at this! It looks good and is > almost ready. I just have some very minor nits I thought I'd bring up.
Thanks for taking a look :) > Paul Bryan <[email protected]> writes: > >> Attached is a patch to fix this. It's essentially just the patch from >> No Wayman. I've added the reported by link. Not sure how if it should >> also be credited somehow. It's only a very little thing though. > > When committing it add the argument "--author="Nicholas Vollmer > <[email protected]>"" to set the author correctly. I > grabbed the value from one of the earlier patches. > > If you've modified it enough to want your own name on it you can always > add "Co-authored-by:". > I don't care if my name is attached or not. Regarding giving credit, would the etiquette be to check with the original author first before attaching their name? I guess they did submit the patch to the list so credit would be appropriate - it just took a few years to be applied! It's only a small patch so maybe it doesn't make much difference either way. >>>>> Ihor Radchenko <[email protected]> writes: >>>>>> >>>>>> I think we can make separator a variable (not a custom option). >>>>>> This way, if someone absolutely wants to change it, they could. > >> +(defconst org-tag-crm-separator "[ \t]*[:,][ \t]*" >> + "Regexp used to determine `crm-separator' when reading multiple tags.") >> + > > A modifiable variable would mean defvar, not defconst. > I went with defconst since the various existing org-tag regexes are of this form and most other defvars in org.el are not initialised. Seemed to be the convention to follow here. As I understand it, the idea is for users to modify it only if they really, really want to. Mostly it should be left as a constant. Happy to defer to others judgement on that though. >> From c8d41be0b18da80cd53437061af3f2a3da766329 Mon Sep 17 00:00:00 2001 >> From: Paul Bryan <[email protected]> >> Date: Tue, 13 Oct 2026 21:50:45 +1100 >> Subject: [PATCH] org.el, org-capture.el: Introduce variable for completing >> read separator reg when reading tags. > > "separator reg"? oops. regex :) > >> * lisp/org.el org-tag-crm-separator: Introduce variable for regex >> determining completing read separator when reading tags. > > Put the variable in parenthesis like "(org-tag-crm-separator)". > Okay, I'll do that. Seem a bit confusing given a sexp like that would mean something like a function or macro. I guess the syntax here is not lisp though! >> Separator now includes commas "," and colons ":". > > Separator has always included colons ":" :) Ha. Indeed! I'll change it to Separator now includes commas "," in addition to colons ":". > >> Reported by: "No Wayman" <[email protected]> > > Use "Reported-by". This is how git commit meta-data stuff works I > think. I've never looked into what it actually does but I think you > need to write them out exactly. Hmm. Didn't spot the missing "-". Will fix. > Although if you change the author of the commit you don't need a > "Reported-by" :) > > > Thanks for helping us with this older report! Do let me know if you > ever need any help with anything at all :) Awesome. I had intended to be more active, but haven't quite managed it yet. It's great that this is such a helpful and supportive community though. I may take you up on the offer some time! -- Paul
