Skip to content

Conversation

@lenhard
Copy link
Member

@lenhard lenhard commented Jul 11, 2016

Implements #1356 and provides a formatter that converts HTML to Unicode (converting characters and eliminating tags)

  • Change in CHANGELOG.md described
  • Tests created for changes

@simonharrer
Copy link
Contributor

LGTM

Cleans_up_LaTeX_code.=Räumt_LaTeX-Code_auf.

Converts_HTML_code_to_LaTeX_code.=Konvertiere_HTML-Code_in_LaTeX-Code.
Converts_HTML_code_to_Unicode.= Konvertiere_HTML-Code_in_Unicode.
Copy link
Member

Choose a reason for hiding this comment

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

Is there a space after the = sign?

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed! Thanks for spotting!

@stefan-kolb
Copy link
Member

LGTM 👍


@Before
public void setUp() throws Exception {
Globals.prefs = JabRefPreferences.getInstance();
Copy link
Member

Choose a reason for hiding this comment

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

It is best practice to init the class under test in the @Before method so that every test method gets a fresh class, i.e move the = new HtmlToUnicodeFormatter part to this method.

Copy link
Member Author

Choose a reason for hiding this comment

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

You are correct. That comes from a copy paste from the other formatter tests, which all share this problem. I will fix all of them in this PR.

@tobiasdiez
Copy link
Member

tobiasdiez commented Jul 11, 2016

I have only a few small remarks. 👍 for merge in general

@lenhard lenhard merged commit 190497d into master Jul 13, 2016
@stefan-kolb stefan-kolb deleted the implement-1356 branch July 13, 2016 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants