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.

Sign up for GitHub

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

Jump to bottom

Add filteringCharset = 'UTF-8' to gradle #1945

Merged
merged 1 commit into from
Sep 11, 2016
Merged

Add filteringCharset = 'UTF-8' to gradle #1945

merged 1 commit into from
Sep 11, 2016

Conversation

Siedlerchr
Copy link
Member

@Siedlerchr Siedlerchr commented Sep 9, 2016 •
edited
Loading

When I read it correct in the doc, this should help fixing the Unicode problems in #1178
I tested locally and it seemed working for me.

It's a good practice to specify the charset when reading and writing the file, using the filteringCharset property. If not specified, the JVM default charset is used, which might not match with the actual charset of the files to filter, and might be different from one machine to another.

https://docs.gradle.org/current/userguide/working_with_files.html#sec:filtering_files

@Siedlerchr
Add filteringCharset = 'UTF-8'
58cbadb 
Convert tabs to spaces
@Siedlerchr Siedlerchr added ui build-system status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers labels Sep 9, 2016
@Siedlerchr Siedlerchr mentioned this pull request Sep 9, 2016
Merged
4 tasks
@Siedlerchr Siedlerchr changed the title Add filteringCharset = 'UTF-8' to gralde Add filteringCharset = 'UTF-8' to gradle Sep 9, 2016
@grimes2
Copy link
Contributor

grimes2 commented Sep 10, 2016

On my system the encoding is now working.

encoding

I changed
filteringCharset = 'UTF-8'
to
filteringCharset = 'ISO-8859-1'

JabRef 3.7dev
windows 10 10.0 amd64
Java 1.8.0_101

@koppor
Copy link
Member

koppor commented Sep 11, 2016

Checking @Siedlerchr's changes, it works. @grimes2 I don't get, why ISO-8859-1 works at your side and UTF-8 doesn't. I just merge this in and we can continue evaluating afterwards.

@koppor koppor merged commit 87a239d into master Sep 11, 2016
@koppor koppor deleted the gradleEncoding branch September 11, 2016 10:46
koppor
koppor reviewed Sep 11, 2016
View reviewed changes
build.gradle
@@ -47,7 +47,7 @@ install4j {
}

task wrapper(type: Wrapper) {
gradleVersion = '2.14.1'
gradleVersion = '3.0'
Copy link
Member

Choose a reason for hiding this comment

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

This is not necessary anymore. See https://gradle.org/blog/gradle-3-0-m1-unleash-the-daemon/

./gradlew wrapper --gradle-version 3.0 has the same effect as ./gradlew wrapper with that config.

I'll just change it directly in the master branch.

Siedlerchr added a commit that referenced this pull request Sep 11, 2016
@Siedlerchr
Merge branch 'master' into xmlunit
9be2619 
* master:
  Remove obsolete wrapper task
  Added error dialog when setting invalid main file directory (#1921)
  Add filteringCharset = 'UTF-8' (#1945)
  Include https://github.com/grimes2
  Searchbar across all bib files instead each having its own (#1549)
  Some OO/LO cleanups (#1927)
  Update link
  Removed external dependency in logic (#1934)
Siedlerchr added a commit to Siedlerchr/jabref that referenced this pull request Sep 11, 2016
@Siedlerchr
Merge branch 'master' into cleanupWarnings
21d39fe 
* master:
  Remove obsolete wrapper task
  Added error dialog when setting invalid main file directory (JabRef#1921)
  Add filteringCharset = 'UTF-8' (JabRef#1945)
  Include https://github.com/grimes2
  Searchbar across all bib files instead each having its own (JabRef#1549)
  Some OO/LO cleanups (JabRef#1927)
  Update link
  Removed external dependency in logic (JabRef#1934)
@grimes2
Copy link
Contributor

grimes2 commented Sep 11, 2016

It seems that this bug is not fixed.
wrong_encoding

JabRef 3.7dev--snapshot--2016-09-11--master--f5ed5ac
windows 10 10.0 amd64
Java 1.8.0_101

@Siedlerchr
Copy link
Member Author

Siedlerchr commented Sep 11, 2016 •
edited
Loading

Yes, you are right. Windows 7 x64 the same.
Edit// I try to look deeper into it,

@Siedlerchr Siedlerchr self-assigned this Sep 11, 2016
@Siedlerchr
Copy link
Member Author

I think I found the reason for it. Property.load assumes automatically ISO... charset.
I try with wrapping it in an InputStreamReader
http://stackoverflow.com/a/17852323

@Siedlerchr Siedlerchr mentioned this pull request Sep 11, 2016
Merged
@koppor
Copy link
Member

koppor commented Sep 11, 2016 •
edited
Loading

Strange, that it was not fixed at your side, here on Win 10, 64bit, the umlauts were displayed correctly.

grabbed_20160912-000823

@Siedlerchr
Copy link
Member Author

Siedlerchr commented Sep 12, 2016 •
edited
Loading

@koppor Nope, if you look closely in the middle you have someone named Krzysztof A. Kościuszkiewicz
where the s -Char is not displayed correctly, only a question mark => Latin 1 (supports only a subset of chars). (I recently read an article about a system in the Swiss Government which had the same problems...)

zesaro pushed a commit to zesaro/jabref that referenced this pull request Nov 22, 2016
@Siedlerchr @zesaro
Add filteringCharset = 'UTF-8' (JabRef#1945)
4e13aa6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Siedlerchr Siedlerchr

Labels
build-system status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers ui
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Siedlerchr @grimes2 @koppor