progfolio / doct

DOCT: Declarative Org Capture Templates for Emacs
GNU General Public License v3.0
375 stars 8 forks source link

Change %doct(keyword) syntax #10

Closed progfolio closed 4 years ago

progfolio commented 4 years ago

@floscr @LemonBreezes @fuzzycode @contrun @BumbleBeeBleep @mohkale @arkhan @swflint @ndw

I'm considering introducing a breaking change to doct and I was curious if anyone has any opinions or ideas about it. Recently @jethrokuan and I discussed the possibility of org-roam using doct. The template string replacement syntax in org-roam is %{keyword} vs doct's %doct(keyword). I'm considering switching to %{keyword} in doct, too.

I'll leave this issue open for a week or so before making a decision. As always, I appreciate all of you helping to make doct a better package.

Thanks, Nick Vollmer.

mohkale commented 4 years ago

I don't particularly mind, this seems nicer to write and updating my config will take like 2 mins 👍.

progfolio commented 4 years ago

Yes, I don't foresee this being too painful to upgrade. Something like this should work:

(defun doct-upgrade-expansion-strings ()
  (save-match-data
    (while (re-search-forward "%doct(\\(.*?\\))" nil t)
      (replace-match "%{\\1}"))))
jethrokuan commented 4 years ago

fwiw org-roam currently uses ${}, which is what s.el uses. But I can also add a template converter so it's transparent to the users, while still using doct underneath.

progfolio commented 4 years ago

My apologies, must've had my wires crossed with org-capture-mode's %^{} syntax when I wrote my notes on this. I could just as easily go for ${}.

jethrokuan commented 4 years ago

I think there's good reason to stick to the %{...} syntax, people may be confused where to use each.

progfolio commented 4 years ago

That's a good point. Regardless of which syntax, I think translating it if needed could be trivial. It's just a simple regexp/replace and that regexp is stored in a variable in doct. So even if another package, say org-roam, decided to go with a different syntax that variable could just be rebound internally prior to converting templates.

e.g.

(let* ((doct--expansion-syntax-regexp "${\\(.*?\\)}")
       (org-roam-capture-templates (doct `((;;etc
                                            ))))))
ndw commented 4 years ago

It wouldn't be onerous to translate to the new syntax and it seems a little nicer. If it's possible to make the current syntax raise an error rather than silently fail, that would be nice.

fuzzycode commented 4 years ago

I have no issues with a breaking change, it will be fixed in 2 minutes tops.

I like the new syntax, seems more inline with what I am used to use for string replacement and if it is more consistent with other emacs/org packages I am all for it.

progfolio commented 4 years ago

The new syntax change has been implemented. doct will do its best to automatically convert the old syntax, %doct(KEYWORD), to the new syntax: %{KEYWORD}. It will also warn during conversion so users can address the change in their configurations. The following command can be executed in the buffer containing your declarations and should take care of changing the syntax. It will show a diff so you can confirm changes before saving the buffer as well. I have not included it in doct's source, so you'll have to copy/evaluate it in your Emacs:

(defun doct-upgrade-expansion-strings ()
  "Upgrade doct expansion string syntax in current buffer.
Shows a diff between buffer and file to confirm changes."
  (interactive)
  (require 'diff)
  (save-excursion
    (goto-char (point-min))
    (save-match-data
      (while (re-search-forward "%doct(\\(.*?\\))" nil t)
        (replace-match "%{\\1}"))))
  (diff-buffer-with-file))