drop required/unique field properties

Instead of having required and unique flags for every field, enforce both
requirements on the first field, and neither on the rest. This mirrors the
subject/body format people are used to in note-taking apps. The subject
defines the object being learnt, and the remaining fields represent properties
of that object.

In the past, duplicate checking served two purposes: it quickly notified the
user that they're entering the same fact twice, and it notified the user if
they'd accidentally mistyped a secondary field. The former behaviour is
important for avoiding wasted effort, and so it should be done in real time.
The latter behaviour is not essential however - a typo is not wasted effort,
and it could be fixed in a periodic 'find duplicates' function. Given that
some users ended up with sluggish decks due to the overhead a large number of
facts * a large number of unique fields caused, this seems like a change for
the better.

This also means Anki will let you add notes as long as as the first field has
been filled out. Again, this is not a big deal: Anki is still checking to make
sure one or more cards will be generated, and the user can easily add any
missing fields later.

As a bonus, this change simplifies field configuration somewhat. As the card
layout and field dialogs are a popular point of confusion, the more they can
be simplified, the better.
This commit is contained in:
Damien Elmes 2011-11-24 22:16:03 +09:00
parent daea038aa4
commit b5c0b1f2c7
11 changed files with 40 additions and 135 deletions

View file

@ -263,7 +263,6 @@ crt=?, mod=?, scm=?, dty=?, usn=?, ls=?, conf=?""",
# more card templates # more card templates
self._logRem(ids, REM_NOTE) self._logRem(ids, REM_NOTE)
self.db.execute("delete from notes where id in %s" % strids) self.db.execute("delete from notes where id in %s" % strids)
self.db.execute("delete from nsums where nid in %s" % strids)
# Card creation # Card creation
########################################################################## ##########################################################################
@ -368,24 +367,18 @@ select id from notes where id in %s and id not in (select nid from cards)""" %
return self.db.execute( return self.db.execute(
"select id, mid, flds from notes where id in "+snids) "select id, mid, flds from notes where id in "+snids)
def updateFieldCache(self, nids, csum=True): def updateFieldCache(self, nids):
"Update field checksums and sort cache, after find&replace, etc." "Update field checksums and sort cache, after find&replace, etc."
snids = ids2str(nids) snids = ids2str(nids)
r = [] r = []
r2 = []
for (nid, mid, flds) in self._fieldData(snids): for (nid, mid, flds) in self._fieldData(snids):
fields = splitFields(flds) fields = splitFields(flds)
model = self.models.get(mid) model = self.models.get(mid)
if csum: r.append((stripHTML(fields[self.models.sortIdx(model)]),
for f in model['flds']: fieldChecksum(fields[0]),
if f['uniq'] and fields[f['ord']]: nid))
r.append((nid, mid, fieldChecksum(fields[f['ord']]))) # apply, relying on calling code to bump usn+mod
r2.append((stripHTML(fields[self.models.sortIdx(model)]), nid)) self.db.executemany("update notes set sfld=?, csum=? where id=?", r)
if csum:
self.db.execute("delete from nsums where nid in "+snids)
self.db.executemany("insert into nsums values (?,?,?)", r)
# rely on calling code to bump usn+mod
self.db.executemany("update notes set sfld = ? where id = ?", r2)
# Q/A generation # Q/A generation
########################################################################## ##########################################################################

View file

@ -87,7 +87,7 @@ class Anki2Importer(Importer):
continue #raise Exception("merging notes nyi") continue #raise Exception("merging notes nyi")
# add to col # add to col
self.dst.db.executemany( self.dst.db.executemany(
"insert or replace into notes values (?,?,?,?,?,?,?,?,?,?,?)", "insert or replace into notes values (?,?,?,?,?,?,?,?,?,?,?,?)",
add) add)
self.dst.updateFieldCache(dirty) self.dst.updateFieldCache(dirty)
self.dst.tags.registerNotes(dirty) self.dst.tags.registerNotes(dirty)

View file

@ -36,8 +36,6 @@ defaultModel = {
defaultField = { defaultField = {
'name': "", 'name': "",
'ord': None, 'ord': None,
'req': False,
'uniq': False,
'sticky': False, 'sticky': False,
# the following alter editing, and are used as defaults for the # the following alter editing, and are used as defaults for the
# template wizard # template wizard

View file

@ -51,13 +51,14 @@ from notes where id = ?""", self.id)
self.usn = self.col.usn() self.usn = self.col.usn()
sfld = stripHTML(self.fields[self.col.models.sortIdx(self._model)]) sfld = stripHTML(self.fields[self.col.models.sortIdx(self._model)])
tags = self.stringTags() tags = self.stringTags()
csum = fieldChecksum(self.fields[0])
res = self.col.db.execute(""" res = self.col.db.execute("""
insert or replace into notes values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)""", insert or replace into notes values (?,?,?,?,?,?,?,?,?,?,?,?)""",
self.id, self.guid, self.mid, self.did, self.id, self.guid, self.mid, self.did,
self.mod, self.usn, tags, self.mod, self.usn, tags,
self.joinedFields(), sfld, self.flags, self.data) self.joinedFields(), sfld, csum, self.flags,
self.data)
self.id = res.lastrowid self.id = res.lastrowid
self.updateFieldChecksums()
self.col.tags.register(self.tags) self.col.tags.register(self.tags)
if self.model()['cloze']: if self.model()['cloze']:
self._clozePostFlush() self._clozePostFlush()
@ -65,18 +66,6 @@ insert or replace into notes values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)""",
def joinedFields(self): def joinedFields(self):
return joinFields(self.fields) return joinFields(self.fields)
def updateFieldChecksums(self):
self.col.db.execute("delete from nsums where nid = ?", self.id)
d = []
for (ord, conf) in self._fmap.values():
if not conf['uniq']:
continue
val = self.fields[ord]
if not val:
continue
d.append((self.id, self.mid, fieldChecksum(val)))
self.col.db.executemany("insert into nsums values (?, ?, ?)", d)
def cards(self): def cards(self):
return [self.col.getCard(id) for id in self.col.db.list( return [self.col.getCard(id) for id in self.col.db.list(
"select id from cards where nid = ? order by ord", self.id)] "select id from cards where nid = ? order by ord", self.id)]
@ -139,51 +128,22 @@ insert or replace into notes values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)""",
# duplicates will be stripped on save # duplicates will be stripped on save
self.tags.append(tag) self.tags.append(tag)
# Unique/duplicate checks # Unique/duplicate check
################################################## ##################################################
def fieldUnique(self, name): def dupeOrEmpty(self):
(ord, conf) = self._fmap[name] "True if first field is a duplicate or is empty."
if not conf['uniq']: val = self.fields[0]
return True if not val.strip():
val = self[name]
if not val:
return True return True
csum = fieldChecksum(val) csum = fieldChecksum(val)
if self.id: # find any matching csums and compare
lim = "and nid != :nid" for flds in self.col.db.list(
else: "select flds from notes where csum = ? and id != ? and mid = ?",
lim = "" csum, self.id or 0, self.mid):
nids = self.col.db.list( if splitFields(flds)[0] == self.fields[0]:
"select nid from nsums where csum = ? and nid != ? and mid = ?", return True
csum, self.id or 0, self.mid) return False
if not nids:
return True
# grab notes with the same checksums, and see if they're actually
# duplicates
for flds in self.col.db.list("select flds from notes where id in "+
ids2str(nids)):
fields = splitFields(flds)
if fields[ord] == val:
return False
return True
def fieldComplete(self, name, text=None):
(ord, conf) = self._fmap[name]
if not conf['req']:
return True
return self[name]
def problems(self):
d = []
for (k, (ord, conf)) in self._fmap.items():
if not self.fieldUnique(k):
d.append((ord, "unique"))
elif not self.fieldComplete(k):
d.append((ord, "required"))
else:
d.append((ord, None))
return [x[1] for x in sorted(d)]
# Flushing cloze notes # Flushing cloze notes
################################################## ##################################################

View file

@ -13,8 +13,6 @@ def addBasicModel(col):
mm = col.models mm = col.models
m = mm.new(_("Basic")) m = mm.new(_("Basic"))
fm = mm.newField(_("Front")) fm = mm.newField(_("Front"))
fm['req'] = True
fm['uniq'] = True
mm.addField(m, fm) mm.addField(m, fm)
fm = mm.newField(_("Back")) fm = mm.newField(_("Back"))
mm.addField(m, fm) mm.addField(m, fm)
@ -34,8 +32,6 @@ def addClozeModel(col):
mm = col.models mm = col.models
m = mm.new(_("Cloze")) m = mm.new(_("Cloze"))
fm = mm.newField(_("Text")) fm = mm.newField(_("Text"))
fm['req'] = True
fm['uniq'] = True
mm.addField(m, fm) mm.addField(m, fm)
fm = mm.newField(_("Notes")) fm = mm.newField(_("Notes"))
mm.addField(m, fm) mm.addField(m, fm)

View file

@ -90,15 +90,11 @@ create table if not exists notes (
tags text not null, tags text not null,
flds text not null, flds text not null,
sfld integer not null, sfld integer not null,
csum integer not null,
flags integer not null, flags integer not null,
data text not null data text not null
); );
create table if not exists nsums (
nid integer not null,
mid integer not null,
csum integer not null
);
create table if not exists cards ( create table if not exists cards (
id integer primary key, id integer primary key,
nid integer not null, nid integer not null,
@ -177,7 +173,6 @@ create index if not exists ix_cards_nid on cards (nid);
create index if not exists ix_cards_sched on cards (did, queue, due); create index if not exists ix_cards_sched on cards (did, queue, due);
-- revlog by card -- revlog by card
create index if not exists ix_revlog_cid on revlog (cid); create index if not exists ix_revlog_cid on revlog (cid);
-- field uniqueness check -- field uniqueness
create index if not exists ix_nsums_nid on nsums (nid); create index if not exists ix_notes_csum on notes (csum);
create index if not exists ix_nsums_csum on nsums (csum);
""") """)

View file

@ -144,7 +144,6 @@ select count() from notes where id not in (select distinct nid from cards)""")
self.col.db.scalar("select count() from cards"), self.col.db.scalar("select count() from cards"),
self.col.db.scalar("select count() from notes"), self.col.db.scalar("select count() from notes"),
self.col.db.scalar("select count() from revlog"), self.col.db.scalar("select count() from revlog"),
self.col.db.scalar("select count() from nsums"),
self.col.db.scalar("select count() from graves"), self.col.db.scalar("select count() from graves"),
len(self.col.models.all()), len(self.col.models.all()),
len(self.col.tags.all()), len(self.col.tags.all()),
@ -188,7 +187,7 @@ select id, nid, did, ord, mod, %d, type, queue, due, ivl, factor, reps,
lapses, left, edue, flags, data from cards where %s""" % d) lapses, left, edue, flags, data from cards where %s""" % d)
else: else:
return x(""" return x("""
select id, guid, mid, did, mod, %d, tags, flds, '', flags, data select id, guid, mid, did, mod, %d, tags, flds, '', '', flags, data
from notes where %s""" % d) from notes where %s""" % d)
def chunk(self): def chunk(self):
@ -356,7 +355,7 @@ from notes where %s""" % d)
def mergeNotes(self, notes): def mergeNotes(self, notes):
rows = self.newerRows(notes, "notes", 4) rows = self.newerRows(notes, "notes", 4)
self.col.db.executemany( self.col.db.executemany(
"insert or replace into notes values (?,?,?,?,?,?,?,?,?,?,?)", "insert or replace into notes values (?,?,?,?,?,?,?,?,?,?,?,?)",
rows) rows)
self.col.updateFieldCache([f[0] for f in rows]) self.col.updateFieldCache([f[0] for f in rows])

View file

@ -177,7 +177,7 @@ select id, id, modelId, 1, cast(created*1000 as int), cast(modified as int),
# and put the facts into the new table # and put the facts into the new table
db.execute("drop table facts") db.execute("drop table facts")
_addSchema(db, False) _addSchema(db, False)
db.executemany("insert into notes values (?,?,?,?,?,?,?,?,'',0,'')", data) db.executemany("insert into notes values (?,?,?,?,?,?,?,?,'','',0,'')", data)
db.execute("drop table fields") db.execute("drop table fields")
# cards # cards
@ -349,15 +349,12 @@ insert or replace into col select id, cast(created as int), :t,
flds = [] flds = []
# note: qsize & qcol are used in upgrade then discarded # note: qsize & qcol are used in upgrade then discarded
for c, row in enumerate(db.all(""" for c, row in enumerate(db.all("""
select name, features, required, "unique", select name, features, quizFontFamily, quizFontSize, quizFontColour,
quizFontFamily, quizFontSize, quizFontColour, editFontSize from fieldModels editFontSize from fieldModels where modelId = ?
where modelId = ?
order by ordinal""", mid)): order by ordinal""", mid)):
conf = dconf.copy() conf = dconf.copy()
(conf['name'], (conf['name'],
conf['rtl'], conf['rtl'],
conf['req'],
conf['uniq'],
conf['font'], conf['font'],
conf['qsize'], conf['qsize'],
conf['qcol'], conf['qcol'],

View file

@ -41,7 +41,6 @@ def test_delete():
assert deck.noteCount() == 0 assert deck.noteCount() == 0
assert deck.db.scalar("select count() from notes") == 0 assert deck.db.scalar("select count() from notes") == 0
assert deck.db.scalar("select count() from cards") == 0 assert deck.db.scalar("select count() from cards") == 0
assert deck.db.scalar("select count() from nsums") == 0
assert deck.db.scalar("select count() from revlog") == 0 assert deck.db.scalar("select count() from revlog") == 0
assert deck.db.scalar("select count() from graves") == 2 assert deck.db.scalar("select count() from graves") == 2

View file

@ -71,27 +71,14 @@ def test_noteAddDelete():
c0 = f.cards()[0] c0 = f.cards()[0]
assert re.sub("</?.+?>", "", c0.q()) == u"three" assert re.sub("</?.+?>", "", c0.q()) == u"three"
# it should not be a duplicate # it should not be a duplicate
for p in f.problems(): assert not f.dupeOrEmpty()
assert not p # now let's make a duplicate
# now let's make a duplicate and test uniqueness
f2 = deck.newNote() f2 = deck.newNote()
f2.model()['flds'][1]['req'] = True
f2['Front'] = u"one"; f2['Back'] = u"" f2['Front'] = u"one"; f2['Back'] = u""
p = f2.problems() assert f2.dupeOrEmpty()
assert p[0] == "unique" # empty first field should not be permitted either
assert p[1] == "required" f2['Front'] = " "
# try delete the first card assert f2.dupeOrEmpty()
cards = f.cards()
id1 = cards[0].id; id2 = cards[1].id
assert deck.cardCount() == 4
assert deck.noteCount() == 2
deck.remCards([id1])
assert deck.cardCount() == 3
assert deck.noteCount() == 2
# and the second should clear the note
deck.remCards([id2])
assert deck.cardCount() == 2
assert deck.noteCount() == 1
def test_fieldChecksum(): def test_fieldChecksum():
deck = getEmptyDeck() deck = getEmptyDeck()
@ -99,31 +86,12 @@ def test_fieldChecksum():
f['Front'] = u"new"; f['Back'] = u"new2" f['Front'] = u"new"; f['Back'] = u"new2"
deck.addNote(f) deck.addNote(f)
assert deck.db.scalar( assert deck.db.scalar(
"select csum from nsums") == int("c2a6b03f", 16) "select csum from notes") == int("c2a6b03f", 16)
# empty field should have no checksum
f['Front'] = u""
f.flush()
assert deck.db.scalar(
"select count() from nsums") == 0
# changing the val should change the checksum # changing the val should change the checksum
f['Front'] = u"newx" f['Front'] = u"newx"
f.flush() f.flush()
assert deck.db.scalar( assert deck.db.scalar(
"select csum from nsums") == int("302811ae", 16) "select csum from notes") == int("302811ae", 16)
# turning off unique and modifying the note should delete the sum
m = f.model()
m['flds'][0]['uniq'] = False
deck.models.save(m)
f.flush()
assert deck.db.scalar(
"select count() from nsums") == 0
# and turning on both should ensure two checksums generated
m['flds'][0]['uniq'] = True
m['flds'][1]['uniq'] = True
deck.models.save(m)
f.flush()
assert deck.db.scalar(
"select count() from nsums") == 2
def test_selective(): def test_selective():
deck = getEmptyDeck() deck = getEmptyDeck()

View file

@ -62,7 +62,7 @@ def test_changedSchema():
def test_sync(): def test_sync():
def check(num): def check(num):
for d in deck1, deck2: for d in deck1, deck2:
for t in ("revlog", "notes", "cards", "nsums"): for t in ("revlog", "notes", "cards"):
assert d.db.scalar("select count() from %s" % t) == num assert d.db.scalar("select count() from %s" % t) == num
assert len(d.models.all()) == num*2 assert len(d.models.all()) == num*2
# the default deck and config have an id of 1, so always 1 # the default deck and config have an id of 1, so always 1