ads-plugin saves pages as extra field instead of pages

Thanks for your patch, I committed it to to trunk: http://bazaar.launchpad.net/~referencer-devs/referencer/trunk/revision/913

I changed the month field to "month" with lowercase "m", I believe this is the standard bibtex field.

It doesn't seem to make a difference whether a field starts with lower or upper case, it is always created as upper case even if it's called as lower case in fields.append . Don't know if that is intentional or not. This is contrast to when you create an extra field by hand in the gui, in that case the field name is created case sensitive. That seems rather inconsistent.

Anyway, I just noticed you may also want to delete line 93 in ads.py which is a todo-comment on adding the year and month fields.

Sorry about my previous comment on the upper/lower case issue. I must have been confused and made a mistake: It does create upper/lower case field names accordingly.

I removed the comment. If you want to do more work on the ads plugin I could give you access to commit directly to trunk?

On 20-01-13 03:04, Mads Chr. Olesen wrote:
> I removed the comment. If you want to do more work on the ads plugin I
> could give you access to commit directly to trunk?
>
Yes that could be handy, I might add another few fields.

I also looked back into my confusing remark on the upper/lower case
field names. I was not entirely wrong in what I said, but it seems to be
an issue specific to the "paste bibtex" feature. Before I was using the
ads-plugin I used to obtain the bibtex entry from ads and used the
"paste bibtex" feature in the properties dialog box in referencer to
import these data. Though the fields in the bibtex record are entirely
lower case, the fields that are not standard in referencer and which
thus end up as extra fields get assigned field names starting with an
upper case. It was thus for consistency with documents that I imported
earlier this way that I choose for the "Month" field instead of "month"
field. I agree it would be better to use the "month" field name, but
then "paste bibtex" should behave that way as well. Do you want me to
file a different bug report for this?

I think that the "paste bibtex" feature needs to cope with all sorts of garbage, so it should somehow try to normalise the case of keys. I'm not sure to what extent it does so currently, it looks like it just relies on the internal BibUtils library -- in which case it might make more sense to make the modifications there.

I looked back into the case issue. It seems bibtex itself is case insensitive, so in that sense it does not really make any difference how the case is normalised in referencer. In fact referencer properly avoids conflicts between keys with different case.
line 731 of Document.C is a comment referring to that, though I could not really identify the piece of code that actually establishes this (my programming knowledge is rather limited). As an example: if there is already a field "mOnth" with value "something" then both "paste bibtex" and the ads-plugin replace "something" with the actual value rather than creating an additional "Month" or "month" field.
Currently, the BibUtils library indeed normalises the case of newly created extra fields through a function firstCap (doing the obvious thing) (see lines 340-341 of BibUtils.C).
It seems to me the case normalisation in the BibUtils library was clearly thought through so I would suggest to try to stick to that and therefore replace the "month" key in the ads-plugin back into a "Month" key.
For further consistency I would suggest to also normalise the case if a field is added through the Document Properties dialog, which is not so at the moment. This just requires calling the firstCap function there as well (line 379 in DocumentProperties.C).

I'm happy to commit the above modifications if you agree.

Yes, I think I agree with your proposed changes.

I made the proposed changes and added an additional check when creating
fields through the document properties dialogue.
However, I was unable to commit to trunk directly (bzr: ERROR: Cannot
lock LockDir(chroot-78667536:///%2Bbranch/referencer/.bzr/branch/lock):
Transport operation not possible: readonly transport)
as you proposed.

Please find the changes in a merge request.

On 14-02-13 06:26, Mads Chr. Olesen wrote:
> Yes, I think I agree with your proposed changes.
>

Sorry for not getting around to it earlier, but your changes are now merged into trunk.

Thanks.

Op 16-04-13 11:50, Mads Chr. Olesen schreef:
> Sorry for not getting around to it earlier, but your changes are now
> merged into trunk.
>