search

koppor / jabref

Collection of simple for JabRef issues. Please submit PRs to https://github.com/jabRef/jabref/.
https://github.com/jabRef/jabref/
MIT License
8 stars 13 forks source link

WIP: Improve reversibility rebased 03 #496

Open antalk2 opened 3 years ago

k3KAW8Pnf7mkmdSMPHz27 commented 3 years ago

I think ManageCitationsDialog and Backend53 are the main missing

Is still true? If that is the case, I'd suggest that we bring the code style more in line with the rest of the JabRef code. BackEnd53 and the migration would probably be better in a separate PR.

antalk2 commented 3 years ago

I think ManageCitationsDialog and Backend53 are the main missing

Is still true?

They are still missing, yes. Just finished separation of formatting the bibliography from inserting it. Started moving stuff to model.

If that is the case, I'd suggest that we bring the code style more in line with the rest of the JabRef code.

BackEnd53 and the migration would probably be better in a separate PR.

ok. To see your comments in the code, I pull from antalk/jabref?

antalk2 commented 3 years ago

add formatBibliography 31b3b38 has inifinite recursion Tried to push, that failed. Tried to pull, now shows a lot of conflicts. Did you mass-format the code? Or did I do something like pulling from the wrong location?

k3KAW8Pnf7mkmdSMPHz27 commented 3 years ago

Comments are not added yet. You’ll be able to see them on this page I just ran out of time today.

On May 6, 2021 at 17:25:14, antalk2 @.**@.)) wrote:

add formatBibliography 31b3b38(https://github.com/koppor/jabref/commit/31b3b38c5847455c7f5ed479fe5cd53c056eb4df) has inifinite recursion Tried to push, that failed. Tried to pull, now shows a lot of conflicts. Did you mass-format the code? Or did I do something like pulling from the wrong location?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub(https://github.com/koppor/jabref/pull/496#issuecomment-833878061), or unsubscribe(https://github.com/notifications/unsubscribe-auth/AAT2NZ4FTLIQRP2U6CPV5ELTMMCLVANCNFSM44H2SPMA).

k3KAW8Pnf7mkmdSMPHz27 commented 3 years ago

Did you mass-format the code? Or did I do something like pulling from the wrong location?

You managed to get this worked out?

Just a last sanity check for me, the starting point was that it is impossible to "re-merge" split citations in the open office document?

E.g., if you had the in-text citations [1-8] [1-3; 9-11] they separate to [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [1] [11] [2] [3] and it is not (currently) possible to reverse it. Some of it was addressed in https://github.com/JabRef/jabref/pull/7455 but to solve it, there must be quite a few changes to how citations are handled (and possibly there will be more options in the "Manage citations" dialog.

Btw, which code editor is it that you are using? I am just wondering if it is possible to make it display the code style used in most of JabRef's code differently.

antalk2 commented 3 years ago

Did you mass-format the code? Or did I do something like pulling from the wrong location?

You managed to get this worked out? Yes, aborted the merge (I do not know what did I pull). Push failed because augmented pushed commit locally. Merged that.

Just a last sanity check for me, the starting point was that it is impossible to "re-merge" split citations in the open office document?

No, the opposite: in jabref52 you can cite as you like, creating individual citations, (or groups if you select multiple bibentries in jabref). Then you click Merge, to join the consecutive citations (and groups) from [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [1] [11] [2] [3] to [1-8] [1-3; 9-11].

Unless you are ready to print now you have a problem: you cannot remove individual citations from a citation group. There was no way to undo the Merge. You had no "Separate" button in JabRef52. Within Libreoffice the Undo button shows a long list of microoperations. If you are lucky, you can go back to before Merge. If you had many citations, due to the limited length of the Undo list LibreOffice may have already dropped the original state. Terminology: "Merge" in GUI, combineCitations in OOBiBBase. The opposite: "Separate" in GUI, unCombineCitations in OOBibBase. In therm of what I know now: "Merge" joins consecutive citation groups into a larger one. Separate splits a group into smaller ones, each containing a single citation.

Aside: If for simplicity we identify small groups with "citations", the "Merge" could be "Group", and "Separate" could be "UnGroup" (Maybe these were suggested at the time, I chose to minimize the changes and keep "Merge" as it was before). With Merge, combine Separate, unCombine I miss the noun. Later started to use CitationGroup and Citation, these terms can be used in a documentation to describe the state before and after. But then "Group" and "Ungroup" or "join citation groups" "split citation groups" (maybe instead of "split", "explode" or something that suggests breaking into small (minimal) pieces) would be more natural in the GUI and in OOBibBase as well.

Some of it was addressed in JabRef#7455 This added the "Separate" button. That works, but revealed that

  1. A pageInfo, as encoded by JabRef52 is associated to a citation group, not to a citation (where it would be meaningful).
  2. "Merge" when creating the large group (and deleting the small ones) does not clean up the associated pageInfo values (If I remember correctly, does not even try to migrate them).
  3. The "Manage citations" dialog actually work on citation groups. In particular, it allows to edit their associated pageInfo values. If you want to see the actual pageInfo records (including those left around, go to LibreOffice: [File]/[Properties]/[Custom properties] and look for names starting with JRcite

but to solve it, there must be quite a few changes to how citations are handled

Minimally: you move the pageInfo from CitationGroup to Citation. This triggers some further changes:

  1. Change how they are stored (at least: how they are associtated in the document) to (now) citations, instead of citation groups.
  2. Change how are they handled in "Merge" (combineCitations) and "Separate" (unCombineCitations). Now they need no special handling, as they move together with the citations they belong to.
  3. Change how they are inserted into citation markers. The JabRef52 solution was: find the closing parenthesis in (Author 2000; Author2 2001) or Author2 (2001) or (1-5) and insert pageInfo before it. With a pageInfo for possibly each citation in a group this was not possible, so I had to modify the code that generates the citation markers to consider and insert the pageInfo fields.
  4. "Manage citations" dialog.

    (and possibly there will be more options in the "Manage citations" dialog. Not in the sense of GUI buttons. 4a) The reference to the associated citation group needs to be replaced or extended to point to the (now) associated citation. 4b) The context taken from the text "As have been already discussed in [1-7], this is an important..." may be sufficient for a cittaion group, but not for a citation within. Probably should add the corresponding bibliography entry (may be too long), or an author-year form citation marker (may be ambiguous). A problem with the latter is that for numeric citation styles we do not have a uniquified citation marker at hand, (only "[3]", but that is not informative without looking up in the bibliography) and even if we generate it, the added letter making it unique is out of context. Maybe we could use the citation key: that is unique unless the same source is cited multiple times in the same group.

There is a question I already mentioned: do we have to support working with documents in the old format, or just migrate them to the new format? For the new format a tree-like representation:

    "As have been already discussed in [1-7], this is an important..."
                    ABC2000 | "pp 1-5"
                    CDE2000 | "pp 3-8"
                    ABC2000 | "pp 13-18"

that is context identifying the group and citations below in order, would probably show the structure we are using, possibly could be extended to edit the order of citations within, join multiple occurences of the same source... But this is probably best left to someone with experience in java GUI programming.

In another line of ideas, instead of coding relations into reference mark names and property names, we could try to create comments (as in LibreOffice [Insert]/[Comment]) and put textual code describing the relations there, for example

 JR \citep{ABC2000 "pp 1-5", CDE2000, "pp 3-8", ABC2000 | "pp 13-18"}

This would both allow the user to modify its content without specialized dialogs and keep the citation marker in the text as location for output from our citation marker generator, unlike the representation used by Jabrefconverter which puts the textual code in the document body. An obvious downside to this would be that we are using the comment facility to store our stuff where the user may have wanted to put his own comments to be exported into PDF. But this is definitely a backend change, for future work.

Btw, which code editor is it that you are using? Emacs, "Java//l mode defined in ‘cc-mode.el’" with indentation style set to "user" (that looked most similar to OOBibBase code at the time).

I am just wondering if it is possible to make it display the code style used in most of JabRef's code differently.

Do you mean the improved automatic line breaks you have shown me in IDEA?

In principle "almost anything" could be done using the builtin LISP and if needed some external programs. I have the package "lsp-java" installed (lsp for Language Server Protocol), but did not set it up to connect to an lsp server. I also installed "projectile" that makes emacs aware of the project top directory (In my case the directory containing the directory of the jabref repository, so I could put a Makefile there with some rules that mostly call gradlew within jabref.) lsp-java could use "Eclipse JDT Language Server". The screen shot https://github.com/emacs-lsp/lsp-java/blob/master/images/demo.png suggests the authors attempt a layout resembling IDEA (and probably many other IDEs)

On the line breaks you already know what do I think: dogmatically avoiding linebreaks on one side and totally giving up on line length limits on the other side is not a good idea. Some rules (of thumb) that individually and in general are good or OK, when taken together and/or too literally may become counterproductive. Accordingly, most tools allow escaping or even turning off the general rules. (For example checkstyle allowed turning off the linelength check, and on the others. The actual combination used is a project-local decision, as well as the decisions concerning other aspects of "expected style".)

Now that you mentioned, starting from https://www.emacswiki.org/emacs/LineWrap I found out about adative-wrap and visual-line-mode. Together they can: break on white space, indent according to the original line and for comments insert // or * for the second line. Otherwise they do not understand java syntax, do not try to indent arguments to the function calls "(" etc. and if there is no space, they will wrap at the edge.

The projects spacing rules, that insist on spaces around operators "a + b", not "a+b" and do not allow spaces at the parenteses: "f(a + b)", not "f( a+b )" have an effect on considered breaking points:

nameOfOThing.nameOfAMethod(widthOfAThing / heightOfAThing).andSomeMore()

the code style used in most of JabRef's code

That is harder to discern, than one would hope: my first impression from OOBibBase was a preference for monolithic classes. Later I revised, in some places there is a tendency towards minimal classes. And there is also a lot of "we would write it differently now, but do not update old code".

k3KAW8Pnf7mkmdSMPHz27 commented 3 years ago

There is a question I already mentioned: do we have to support working with documents in the old format, or just migrate them to the new format? For the new format a tree-like representation:

I'd say that we don't need to support them, just make sure to warn and get a confirmation from the user before migrating.

That is harder to discern, than one would hope: my first impression from OOBibBase was a preference for monolithic classes. Later I revised, in some places there is a tendency towards minimal classes. And there is also a lot of "we would write it differently now, but do not update old code".

There is more to it than that. Someone has to do the code modernization and it is hard to find time for it (or volunteers). I have a personal vendetta against AuthorListParser.java and I wish I'll have the time to modernize it at some point in the future, but meanwhile, we can try to ensure that when code gets added/updated the code standard is also improved.