Comment 1 for bug 605589

Revision history for this message
Janne Kokkala (jikokkala) wrote :

In DocumentProperties::setupFields there are lines which seem to be meant to fix this bug:

/* [bert] Minor change to actually implement and register
* a callback for changes to the DOI field. This is a bit
* ugly here since it assumes we know whether DOI is
* required or not.
*/
if (it->internalName_ == "doi") {
  entry->signal_changed().connect(sigc::mem_fun(*this, &DocumentProperties::onDoiEntryChanged));
}

However, this isn't currently working correctly. Right now the function contains four loops for creating the form for editing metadata:
 1. Long fields taking the whole row; loops over requiredFields, skips short fields
 2. Long fields taking the whole row; loops over optionalFields, skips short fields
 3. Short fields taking only half of the row; loops over requiredFields, skips long fields
 4. Short fields taking only half of the row; loops over optionalFields, skips long fields

The lines above meant to fix the DOI update are in the loop 1. Moving the lines to the loop 2 fixes the bug, but as mentioned in the comment, this is still a bit ugly.

The loops 1 and 2 are identical, and loops 3 and 4 are identical. Perhaps making a new vector allFields by concatenating requiredFields and optionalFields would allow reducing 1&2 to one loop and 3&4 to one loop, and partially get rid of the ugliness of this. However, still some code would need to be moved if one decides to make DOI a short field instead of a long field. It is possible to sort allFields according to the field length, combine all loops into one, and add a few lines to handle the columns correctly, but I'm not sure if this is the best way to do this.