Skip to content

Add compatibility check for UTF8#370

Merged
koppor merged 8 commits into
masterfrom
fix-UTF8-in-OpenDatabaseAction
Nov 20, 2015
Merged

Add compatibility check for UTF8#370
koppor merged 8 commits into
masterfrom
fix-UTF8-in-OpenDatabaseAction

Conversation

@lenhard

@lenhard lenhard commented Nov 19, 2015

Copy link
Copy Markdown
Member

This PR attempts to fix #367

We have a bit of a problem. UTF8 came from these lines

        try (InputStreamReader reader = openFile(fileToOpen, suppliedEncoding, fallbackEncoding)) {
            BibtexParser bp = new BibtexParser(reader);

            ParserResult pr = bp.parse();
            pr.setEncoding(reader.getEncoding());

It is set in this method call. That's the Java API. The InputStreamReader says that the proper name of the encoding is UTF8.

My solution is an ugly compatibility hack in which we just insert the - manually. Ugly, but works... What do you think?

@tobiasdiez

tobiasdiez commented Nov 19, 2015

Copy link
Copy Markdown
Member

What about replacing all string representation of encodings by Charset? Then on export getEncoding.name() returns the correct description "UTF-8".

@lenhard

lenhard commented Nov 19, 2015

Copy link
Copy Markdown
Member Author

Sounds like a good idea! We should give it a try.

@lenhard

lenhard commented Nov 20, 2015

Copy link
Copy Markdown
Member Author

@tobiasdiez great advice! Charset already implements the correct encoding name translation. All we have to do is through it into Charset and read it our again and we can skip the hack:

pr.setEncoding(Charset.forName(reader.getEncoding()).name());

@lenhard lenhard added this to the v3.0 milestone Nov 20, 2015
@koppor

koppor commented Nov 20, 2015

Copy link
Copy Markdown
Member

Please merge master: We did a mayor overhaul of the OpenDatabaseAction: Deleted code etc.

@tobiasdiez

tobiasdiez commented Nov 20, 2015

Copy link
Copy Markdown
Member

@lenhard Glad this was helpful. I think the code would even further improve if the ParserResult (and maybe some other classes) doesn't hold the encoding as a string but as a Charset, i.e. public Charset getEncoding() and similarly for setEncoding.

@lenhard
Merge branch 'master' into fix-UTF8-in-OpenDatabaseAction
567227b 
Conflicts:
	src/main/java/net/sf/jabref/importer/OpenDatabaseAction.java
@lenhard
Redo refactorings in OpenDatabaseAction
6006f80
@lenhard

lenhard commented Nov 20, 2015

Copy link
Copy Markdown
Member Author

@koppor Done!
@tobiasdiez Sure. But I think this should be part of a new PR, since this PR just attempts to fix one particular issue.

koppor added a commit that referenced this pull request Nov 20, 2015
@koppor
Merge pull request #370 from JabRef/fix-UTF8-in-OpenDatabaseAction
faa36bd 
Add compatibility check for UTF8
@koppor koppor merged commit faa36bd into master Nov 20, 2015
@stefan-kolb stefan-kolb deleted the fix-UTF8-in-OpenDatabaseAction branch November 20, 2015 16:03
tobiasdiez added a commit to tobiasdiez/jabref that referenced this pull request Nov 26, 2015
@tobiasdiez
Change representation of encodings to Charset
377200d 
Instead of representing an encoding as a string we now use a Charset.
This idea was already discussed in PR JabRef#370.
tobiasdiez added a commit to tobiasdiez/jabref that referenced this pull request Nov 26, 2015
@tobiasdiez
Change representation of encodings to Charset
f7385c3 
Instead of representing an encoding as a string we now use a Charset.
This idea was already discussed in PR JabRef#370.
@tobiasdiez tobiasdiez mentioned this pull request Nov 26, 2015
Merged
tobiasdiez added a commit to tobiasdiez/jabref that referenced this pull request Nov 30, 2015
@tobiasdiez
Change representation of encodings to Charset
c0ee130 
Instead of representing an encoding as a string we now use a Charset.
This idea was already discussed in PR JabRef#370.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v3.0

Development

Successfully merging this pull request may close these issues.

UTF-8 instead of UTF8

3 participants

@lenhard @tobiasdiez @koppor