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

[WIP] LaTeX Integration project #5011

Open
wants to merge 7 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@davidemdot
Copy link
Member

commented May 31, 2019

Hey! This is the pull request where I will be uploading everything related to the LaTeX Integration project, which Tobias mentioned in #5002.

For the moment, I have been coding the backend of the TEX parser and preparing tests for these classes. Before the end of this week, I will add support for nested files and cross-references and will keep improving the related documentation.

Any feedback is more than welcome! :"D

@davidemdot davidemdot changed the title LaTeX Integration project [WIP] LaTeX Integration project May 31, 2019

@Siedlerchr Siedlerchr added the GSoC label May 31, 2019

@davidemdot davidemdot closed this May 31, 2019

@davidemdot davidemdot reopened this May 31, 2019

@tobiasdiez
Copy link
Member

left a comment

That looks already nice. Good job! I've added a few remarks.

In general, I would also to propose to add a parse(String) method, which you can use in the tests. For example, parse("\parencite[p.4]{RefA}") and then verify that the result is what you expect.

davidemdot added some commits Jun 4, 2019

@tobiasdiez

This comment has been minimized.

Copy link
Member

commented Jun 7, 2019

A suggestion for the UI of the search result window: I like the design of ResearchGate to show the citations
image

We can do something similar, just replacing the reference (Minimal two-spheres ...) with the name of the latex document, and the links ("View") with an option to jump to the citation in the latex file.

In general, tables are nice if you want to compare items while lists are preferred if the user is more interested in the details of an item. I guess for the search result window we are in the second case (although that might differ from user to user).

@tobiasdiez
Copy link
Member

left a comment

The code looks already pretty good. I added a few suggestions to make the tests a bit easier to understand.

@davidemdot

This comment has been minimized.

Copy link
Member Author

commented Jun 10, 2019

Thanks for your feedback, @Siedlerchr and @tobiasdiez! Now all identified issues should be fixed.

  • TODO: Add support for multicite commands.
  • TODO: Expand integration test.
@davidemdot

This comment has been minimized.

Copy link
Member Author

commented Jun 13, 2019

@tobiasdiez, what do you think of this GUI, according to the example that you proposed?

Parser UI

By the way, you all can follow the development of the user interface in other pull request (this one is becoming very big).

@tobiasdiez
Copy link
Member

left a comment

I really like the user interface! Some suggestions that you may try out:

  • Move the "Jump to file" a line up, directly after the file name.
  • Print the file name in normal font instead of in bold (it depends a bit on where you want to draw attention to, either to the citation text or to the file containing the citation)
  • Run the latex2unicode converter on the citation text in the view model (so that commands like \textbf{blaba} are replaced by a bold blaba)

I had a second look at the code here. It looks really good now, and I've only some minor comments. I think we are very close to mark this code as production-ready!

/**
* Look for an equivalent BibTeX entry within the reference database for all keys inside of the TEX files.
*/
public CrossingKeysResult resolveKeys() {

This comment has been minimized.

Copy link
@tobiasdiez

tobiasdiez Jun 14, 2019

Member

If I understand the code correctly, the CrossingKeys class provides a facility to enrich a TexParserResult to a CrossingKeysResult, where the entry keys are resolved. I would make this also clear from the method signature and move the texParserResult argument from the constructor to the resolveKeys method (and remove the getResult method).

private final BibDatabase newDatabase;
private int crossRefsCount;

public CrossingKeysResult(TexParserResult texParserResult, BibDatabase masterDatabase) {

This comment has been minimized.

Copy link
@tobiasdiez

tobiasdiez Jun 14, 2019

Member

Personally, I would remove the masterDatabase from this class.

private final TexParserResult texParserResult;
private final BibDatabase masterDatabase;
private final List<String> unresolvedKeys;
private final BibDatabase newDatabase;

This comment has been minimized.

Copy link
@tobiasdiez

tobiasdiez Jun 14, 2019

Member

What was your reason to use a BibDatabase instead of a simple List<BibEntry> ?

* Insert into the database a clone of the given entry. The cloned entry has a new unique ID.
*/
public void insertEntry(BibEntry entry) {
BibEntry clonedEntry = (BibEntry) entry.clone();

This comment has been minimized.

Copy link
@tobiasdiez

tobiasdiez Jun 14, 2019

Member

Do we really need to clone the entry? That is usually not necessary and might lead to confusions (e.g. when you modify the cloned entry without realizing that it is a clone and thus does not affect the original entry).


private final List<Path> fileList;
private final List<Path> nestedFiles;
private final Map<String, Set<Citation>> citations;

This comment has been minimized.

Copy link
@tobiasdiez
private static BibDatabase database2;

@BeforeEach
private void setUp() {

This comment has been minimized.

Copy link
@tobiasdiez

tobiasdiez Jun 14, 2019

Member

These tests look really nice now. Good job!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.