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

Fix exception if linked file has masked umlauts / invalid characters in path #8851

Merged
merged 13 commits into from
Jun 6, 2022
Merged

Fix exception if linked file has masked umlauts / invalid characters in path #8851

merged 13 commits into from
Jun 6, 2022

Conversation

the-star-sea
Copy link
Contributor

@the-star-sea the-star-sea commented May 26, 2022 •
edited by Siedlerchr
Loading

Fixes #8786

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

the-star-sea
the-star-sea commented May 26, 2022
View reviewed changes
Copy link
Contributor Author

@the-star-sea the-star-sea left a comment

Choose a reason for hiding this comment

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

I have viewed all the changes

@ThiloteE ThiloteE changed the title fix #8786 Exception if linked file has masked umlauts / invalid characters in path Fix exception if linked file has masked umlauts / invalid characters in path May 27, 2022
@Siedlerchr Siedlerchr added the status: changes required Pull requests that are not yet complete label May 27, 2022
@Siedlerchr
Copy link
Member

Siedlerchr commented May 27, 2022 •
edited by koppor
Loading

Thanks for your PR!
I've improved your code a bit: Following the DRY principle, if you encounter duplicate code that does the same, refactor it into a common method.

What still needs to be done is to create an IntegrityCheck for this as well as I wrote in my comment.

@the-star-sea
Copy link
Contributor Author

the-star-sea commented May 27, 2022 •
edited
Loading

Thanks for your PR! I've improved your code a bit: Following the [DRY principle](https://dzone.com/articles/software-design-principles-dry-and-kiss(, if you encounter duplicate code that does the same, refactor it into a common method.

What still needs to be done is to create an IntegrityCheck for this as well as I wrote in my comment.

Thanks for your review.How can I merge the change into my local respository.There shows no change.

@Siedlerchr
Copy link
Member

I pushed them to your branch on your repo. You have to use git fetch and git pull

koppor
koppor requested changes May 31, 2022
View reviewed changes
src/main/java/org/jabref/model/util/FileHelper.java Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
src/test/java/org/jabref/model/util/FileHelperTest.java Outdated Show resolved Hide resolved
@calixtus
Copy link
Member

calixtus commented Jun 4, 2022

Any update here?

@the-star-sea @koppor
Update CHANGELOG.md
ae63e17 
Co-authored-by: Oliver Kopp <[email protected]>
@the-star-sea
Copy link
Contributor Author

Any update here?

I am preparing for my final exams these days.I will do it later

Siedlerchr
Siedlerchr approved these changes Jun 5, 2022
View reviewed changes
Copy link
Member

@Siedlerchr Siedlerchr left a comment

Choose a reason for hiding this comment

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

LGTM! I am not sure about the deletion of the JabRef Main file in .idea

@Siedlerchr Siedlerchr added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers and removed status: changes required Pull requests that are not yet complete labels Jun 5, 2022
@calixtus
Copy link
Member

calixtus commented Jun 5, 2022

Should not be deleted. Was an intentional decision to include intellij configuration to ease workspace setup.

@koppor koppor merged commit 45b40a7 into JabRef:main Jun 6, 2022
koppor
koppor reviewed Jun 6, 2022
View reviewed changes
Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

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

The new method detectBadFileName should have deserved test cases. --> Try to test small parts of the code. Since this PR is opened for more than a week and fixes an annoying issue, this can be done in a follow-up PR.

@koppor koppor removed the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jun 6, 2022
Siedlerchr added a commit that referenced this pull request Jun 23, 2022
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into controlsfx-tags
2e236f0 
* upstream/main: (27 commits)
  Add gitpod config (#8921)
  Fix javafx version not displayed in About Dialog (#8918)
  Redesign "Manage field names & content" dialog (#8892)
  Rework IACR fetcher (#8904)
  Bump h2-mvstore from 2.1.212 to 2.1.214 in /buildSrc (#8915)
  Bump unoloader from 7.3.3 to 7.3.4 (#8912)
  Bump libreoffice from 7.3.3 to 7.3.4 (#8913)
  Bump tika-core from 2.4.0 to 2.4.1 (#8914)
  Bump org.eclipse.jgit from 6.1.0.202203080745-r to 6.2.0.202206071550-r (#8916)
  Squashed 'buildres/csl/csl-styles/' changes from e740261..845dee0 (#8903)
  Bump flowless from 0.6.9 to 0.6.10 (#8898)
  Bump postgresql from 42.3.5 to 42.4.0 (#8900)
  Bump mockito-core from 4.6.0 to 4.6.1 (#8899)
  Bump antlr-runtime from 3.5.2 to 3.5.3 (#8897)
  Update to MADR 3.0.0-beta.2 (#8896)
  Increase the network connection timeout and improve error message (#8894)
  Fix linux terminal opening process error (#8891)
  Fix exception if linked file has masked umlauts / invalid characters in path (#8851)
  Remove obsolte CHANGELOG.md entry
  Revert "Fix right clicking a group and choosing "remove selected entries from this group" leads to error when Bibtex source tab is selected (#8821)"
  ...

# Conflicts:
#	build.gradle
@koppor
Copy link
Member

koppor commented Sep 3, 2022

This PR caused a regression: #8991

We think, we fixed it at #9129 - by allowing the fileName at detectBadFileName being a relative (or absolute) path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@koppor koppor koppor requested changes

@Siedlerchr Siedlerchr Siedlerchr approved these changes

Assignees
No one assigned
Labels
external files import Logging
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Exception if linked file has masked umlauts / invalid characters in path (from Citavi export in my case)
5 participants
@the-star-sea @Siedlerchr @calixtus @koppor @ThiloteE