Skip to content

Improved conversion from LaTeX to HTML/Unicode#841

Merged
simonharrer merged 14 commits into
JabRef:masterfrom
oscargus:bigconversionlist
Feb 26, 2016
Merged

Improved conversion from LaTeX to HTML/Unicode#841
simonharrer merged 14 commits into
JabRef:masterfrom
oscargus:bigconversionlist

Conversation

@oscargus

Copy link
Copy Markdown
Contributor

Now the huge list of conversion symbols is used when converting from LaTeX to HTML and Unicode. I also added a field right-click menu item to convert from LaTeX to Unicode. This sort of works except that it doesn't handle text within $$ in a good way (remove the $$ and the symbol conversion works).

The huge diff is caused by reformatting the huge list, as the newly added entries was aligned differently compared to earlier on saving...

@oscargus oscargus added [outdated] type: enhancement status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers labels Feb 20, 2016
Assert.assertEquals("ı", layout.format("\\i"));
Assert.assertEquals("ı", layout.format("\\{i}"));
Assert.assertEquals("ıı", layout.format("\\i\\i"));
Assert.assertEquals("ı ı", layout.format("\\i \\i"));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does a HTMLChars() formatter format latex to unicode and not to HTML?
Is there something wrong with the implementation or is the naming just suboptimal?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Historical reasons. Earlier the huge conversion list resided in that file, so adding a method "formatUnicode" was simple. Indeed there should (very soon) be a UnicodeToLatexConverter extracted... Doing this is steps...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorry, wrong answer. :-) The answer above holds for HTMLConverter...

I think HTMLChars format to HTML? Now, since there are named entities these are used, if not, the numerical are used.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah my mistake! Sorry....

@oscargus oscargus force-pushed the bigconversionlist branch 2 times, most recently from 1783e9d to aa1ad68 Compare February 20, 2016 12:24
@oscargus

Copy link
Copy Markdown
Contributor Author

I fixed the equation issue and added support for some more LaTeX text styles. For example, \textsuperscript and \textsubscript are supported for both HTML and OO.

This also means that the preview looks quite a bit better now.

@simonharrer

Copy link
Copy Markdown
Contributor

Please fix the failing tests. Then this can be merged in as well.

@simonharrer

Copy link
Copy Markdown
Contributor

Converters could be added to the Save Actions under the Formatter Interface as well.

@oscargus

oscargus commented Feb 22, 2016 via email

Copy link
Copy Markdown
Contributor Author

@oscargus

oscargus commented Feb 22, 2016 via email

Copy link
Copy Markdown
Contributor Author

@simonharrer simonharrer removed the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Feb 22, 2016
@simonharrer

Copy link
Copy Markdown
Contributor

Ok, please add the label ready-for-review again when you are done with the other changes.

@oscargus oscargus added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers and removed status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers labels Feb 22, 2016
@oscargus

Copy link
Copy Markdown
Contributor Author

It turned out that it also made sense to move the layout formatters from export.layout to logic.layout, so the final(?) commit is handling that. Some minor refactoring had to be done, but nothing controversial.

@oscargus oscargus added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Feb 22, 2016
@oscargus oscargus force-pushed the bigconversionlist branch 2 times, most recently from bf49de3 to c3526b8 Compare February 22, 2016 22:50
@oscargus

Copy link
Copy Markdown
Contributor Author

I'm giving up on this PR for now... Doesn't really make sense to add a JournalAbbreviationRepository argument to PdfImporter or MoveFileAction, although that is required if it is propagated (and now I've only gone two steps back...). Will be back after a week or so of holidays (true story, nothing to do with this)...

Accessing a global variable is a single formatter or passing an argument to hundreds of non-related classes to possibly be used in a single formatter? I'm not convinced it is worth it...

@oscargus oscargus removed the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Feb 23, 2016
@simonharrer

Copy link
Copy Markdown
Contributor

The idea is that the GUI part can have globals, whereas the logic does not. With this in mind, the PdfImporter or MoveFileAction do not need to have this class, but can merely pass in the global variable if necessary. Does this make sense?

@oscargus

Copy link
Copy Markdown
Contributor Author

It makes sense from the perspective that I understand where to start/stop. :-)

@oscargus oscargus added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Feb 26, 2016
@simonharrer

Copy link
Copy Markdown
Contributor

👍 LGTM (but can you remove the unused imports, please?)

@oscargus

Copy link
Copy Markdown
Contributor Author

All(?) imports are now removed and I managed to sort out the PdfCleanup thing as well.

Feel free to merge. :-)

simonharrer added a commit that referenced this pull request Feb 26, 2016
Improved conversion from LaTeX to HTML/Unicode
@simonharrer simonharrer merged commit 69e4fe2 into JabRef:master Feb 26, 2016
@tobiasdiez

Copy link
Copy Markdown
Member

Sorry for being such a pain in the ass with this abbreviation thing...but one last remark: did you tried out the Pdf rename cleanup as a user? I think the dialog always passes a null repository to the cleanup worker which in the end would result in a NPE. Not sure through...

@oscargus oscargus deleted the bigconversionlist branch March 16, 2016 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[outdated] type: enhancement status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants