Skip to content

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

Merged
koppor merged 13 commits into
JabRef:mainfrom
tongmd:fix-issue-8786
Jun 6, 2022
Merged

Fix exception if linked file has masked umlauts / invalid characters in path#8851
koppor merged 13 commits into
JabRef:mainfrom
tongmd:fix-issue-8786

Conversation

@tongmd

@tongmd tongmd commented May 26, 2022 •
edited by Siedlerchr
Loading

Copy link
Copy Markdown
Contributor

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.

tongmd
tongmd commented May 26, 2022
View reviewed changes

@tongmd tongmd left a comment

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.

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
Siedlerchr reviewed May 27, 2022
View reviewed changes
Comment thread src/main/java/org/jabref/model/util/FileHelper.java Outdated
@Siedlerchr

Siedlerchr commented May 27, 2022 •
edited by koppor
Loading

Copy link
Copy Markdown
Member

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.

@tongmd @Siedlerchr
Update src/main/java/org/jabref/model/util/FileHelper.java
11f280e 
Co-authored-by: Christoph <siedlerkiller@gmail.com>
@tongmd

tongmd commented May 27, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

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

Siedlerchr commented May 29, 2022

Copy link
Copy Markdown
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
Comment thread src/main/java/org/jabref/model/util/FileHelper.java
Comment thread CHANGELOG.md Outdated
Comment thread src/test/java/org/jabref/model/util/FileHelperTest.java Outdated
@calixtus

calixtus commented Jun 4, 2022

Copy link
Copy Markdown
Member

Any update here?

@tongmd @koppor
Update CHANGELOG.md
ae63e17 
Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>
@tongmd

tongmd commented Jun 4, 2022

Copy link
Copy Markdown
Contributor Author

Any update here?

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

@tongmd tongmd requested review from Siedlerchr and koppor June 4, 2022 13:34
Siedlerchr
Siedlerchr approved these changes Jun 5, 2022
View reviewed changes

@Siedlerchr Siedlerchr left a comment

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.

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

calixtus commented Jun 5, 2022

Copy link
Copy Markdown
Member

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

@koppor koppor left a comment

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.

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

koppor commented Sep 3, 2022

Copy link
Copy Markdown
Member

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

component: external-files component: import-load component: 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

@tongmd @Siedlerchr @calixtus @koppor @ThiloteE