Skip to content
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

Support to extra field in Cloze note type #47

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

scarbajali
Copy link

Quick (and dirty) implementation of #36.

  • Anki plugin: add Extra field to the TiddlyRemember Cloze note type (v2).
  • Tiddlywiki plugin: add extra parameter to the remembercz macro.

@sobjornstad
Copy link
Owner

Thanks for sending this over! I will take a look at it and most likely integrate this into some other changes I'm making when I get a chance to dive into TiddlyRemember stuff. Guessing that will be sometime next week.

Looking at what you have off the top of my head, changing the order of the arguments to the macro in TiddlyWiki isn't a good idea since it will break users with existing positional parameters, and I want to pull in some code from the sister project LPCG to allow updating existing note types in Anki (rather than creating an entirely new note type) when there is a simple, backwards-compatible change like this one. This is a great start though.

@scarbajali
Copy link
Author

I changed the implementation to avoid legacy issues. So I added instead the remembercze macro that contains extra field but it doesn't support inline option, as it looks like that doesn't make sense to have an extra field in this case.

remembercz works as initially conceived, without extra field.

Copy link
Owner

@sobjornstad sobjornstad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marking this as a comment since I need to put together some new code myself before this can be finished up. Feel free to look at the comments other than the note-type-upgrading one anytime, and thanks for working on this!

anki-plugin/src/trmodels.py Outdated Show resolved Hide resolved
@@ -336,8 +340,8 @@ class TiddlyRememberClozeTemplate(TemplateData):
</div>
"""

name = "TiddlyRemember Cloze v1"
fields = ("Text", ID_FIELD_NAME, "Wiki", "Reference", "Permalink")
name = "TiddlyRemember Cloze v2"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like I mentioned, I don't want this to be a separate note type. I'll dive into implementing upgrades over the next couple of days and get back to you.

<$list filter="[[$mode$]!match[inline]]" variable=_>
<$macrocall $name=twRememberClozeBlock id=<<__id__>> text=<<__text__>> reference=<<__reference__>>/>
</$list>
<$macrocall $name=remembercze id=<<__id__>> text=<<__text__>> extra="" mode=<<__mode__>> reference=<<__reference__>>/>
Copy link
Owner

@sobjornstad sobjornstad Sep 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I was expecting to do this differently...rather than creating an entirely new macro, just put the new field at the end of the argument list. Like

\define remembercz(id, text, mode: "block", reference: "", extra:"")

Then it would be backwards compatible and we wouldn't have to add in an extra stub and change the macro button to insert something new. It's still easy to use without specifying a mode or reference...

<<remembercz "20200101120000000" "This is my {test} cloze." extra:"Extra content goes here.">>

Unless you see some reason that doesn't work?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did on this way just to avoid to put the "extra:", but I think it is only trivial detail. Your proposal looks cleaner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants