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.

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

Find Unlinked files should ignore Thumbs.db, etc #8800

Merged
merged 29 commits into from
Jun 27, 2022

Conversation

prashant1712
Copy link
Contributor

@prashant1712 prashant1712 commented May 15, 2022

Fixes JabRef#373.

Reads the .gitignore file in the root directory of the library path and obeys it according to the option selected in the UI dialog box.

  • 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.

Screenshot 2022-05-16 at 5 09 16 AM

CHANGELOG.md Outdated Show resolved Hide resolved
@prashant1712
Copy link
Contributor Author

Thank you for your suggestions. I will make the changes and commit it soon.

.gitignore Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
Copy link
Member

@calixtus calixtus left a comment

Choose a reason for hiding this comment

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

Great work. Some minor remarks, but looks already very good!

.idea/runConfigurations/JabRef_Main.xml Outdated Show resolved Hide resolved
DEFAULT(Localization.lang("Default")),
INCLUDE_ALL(Localization.lang("Include All Files"));

private static FileIgnoreUnlinkedFiles instance;
Copy link
Member

Choose a reason for hiding this comment

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

Is this necessary? You already have a var in FileFilterUtils...

@prashant1712
Copy link
Contributor Author

@calixtus Thank you for the review. We are just finishing on the testing of the changes. We will let you know when it's done.

Check for existing files before reading
@prashant1712
Copy link
Contributor Author

@Siedlerchr Thank you for your help. Really appreciate the refactoring to improve the code, helped us in understanding the code better. One small thing, you changed the code to read the .gitignore file from the system of the user which i don't think is appropriate in our case. I'm changing it back to read it from the source directory of Jabref. Do let us know your opinion on this.

@Siedlerchr
Copy link
Member

Yeah, that's one thing I wasn't sure about. I thought it makes sense to check the folder you selected to search for the files and the subfolders if there is any gitignore file inside and use this then?

@prashant1712
Copy link
Contributor Author

Yeah, i'm not really sure what to do now. Which scenario are we going to take: Searching for the .gitignore file in the directory selected by the user or picking up the .gitignore file from the source directory of JabRef. We initially thought that it makes sense to ignore the extensions and file names mentioned in the .gitignore file of JabRef as it gives a consistent behaviour across all users but I'm quite confused right now and would really appreciate a solution to this.

@Siedlerchr
Copy link
Member

Let's ask @koppor as it was his issue...

Comment on lines 3 to 8
import java.io.BufferedReader;
import java.io.File;
import java.io.IOException;
import java.nio.file.DirectoryStream.Filter;
import java.nio.file.Files;
import java.nio.file.Path;
Copy link
Member

Choose a reason for hiding this comment

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

I just mentioned you are mixing pre-java7-io api and nio. Is there a special reason?

@koppor
Copy link
Member

koppor commented May 23, 2022

Yeah, i'm not really sure what to do now. Which scenario are we going to take: Searching for the .gitignore file in the directory selected by the user

Yes, do this.

In addition following files / directories are always ignored:

  • .git
  • Thumbs.db

This list is hard-coded in Java. No additional text file. No option for configuration.

@prashant1712
Copy link
Contributor Author

Understood. Thank you @koppor for the clarification. I'll make the changes accordingly.

@Siedlerchr
Copy link
Member

Please also include .DS_Store (A kind of desktop.ini equivalent for mac) in the default ignored files

@koppor koppor added the status: changes required Pull requests that are not yet complete label May 31, 2022
@calixtus
Copy link
Member

calixtus commented Jun 4, 2022

Any update here?

@prashant1712
Copy link
Contributor Author

Hi @calixtus, i am recovering from a medical condition right now. Will complete the changes when i'm recovered. Thank you for understanding.

@calixtus
Copy link
Member

calixtus commented Jun 5, 2022

Of course, get well soon.

koppor and others added 2 commits June 20, 2022 20:57
Co-authored-by: Christoph <siedlerkiller@gmail.com>
Co-authored-by: Carl Christian Snethlage <50491877
Co-authored-by: ThiloteE <73715071+ThiloteE@users.noreply.github.com>
@koppor
Copy link
Member

koppor commented Jun 20, 2022

@prashant1712 In each dev call, we choose a PR (more or less randomly) to finish it. Today, we worked 2 hours on this one. the intermediate result is there. However, the tests still fail. We need to continue here.

Siedlerchr and others added 3 commits June 27, 2022 21:53
* upstream/main:
  Try to  make reproducible builds (JabRef#8925)
  Link to new board (and refine text)
  Pass the data as a string (JabRef#8923)
  Add IntelliJ plugins to Gitpod configuration
  Add gradle support
  Fix extension name
  Remove obsolete closing bracket
  Add gitpod config (JabRef#8921)
  Fix javafx version not displayed in About Dialog (JabRef#8918)
  Redesign "Manage field names & content" dialog (JabRef#8892)
  Rework IACR fetcher (JabRef#8904)
  Bump h2-mvstore from 2.1.212 to 2.1.214 in /buildSrc (JabRef#8915)
  Bump unoloader from 7.3.3 to 7.3.4 (JabRef#8912)
  Bump libreoffice from 7.3.3 to 7.3.4 (JabRef#8913)
  Bump tika-core from 2.4.0 to 2.4.1 (JabRef#8914)
  Bump org.eclipse.jgit from 6.1.0.202203080745-r to 6.2.0.202206071550-r (JabRef#8916)
add some more tests
Fix checsktyle
@koppor koppor removed the status: changes required Pull requests that are not yet complete label Jun 27, 2022
@koppor koppor merged commit 2d15917 into JabRef:main Jun 27, 2022
Siedlerchr added a commit to LIM0000/jabref that referenced this pull request Jun 28, 2022
* upstream/main:
  Fix for Dark theme not readable (JabRef#8929)
  Find Unlinked files should ignore Thumbs.db, etc (JabRef#8800)
  Try to  make reproducible builds (JabRef#8925)
  Link to new board (and refine text)
  Pass the data as a string (JabRef#8923)
  Add IntelliJ plugins to Gitpod configuration
  Add gradle support
  Fix extension name
  Remove obsolete closing bracket
  Add gitpod config (JabRef#8921)
  Fix javafx version not displayed in About Dialog (JabRef#8918)
  Redesign "Manage field names & content" dialog (JabRef#8892)
Siedlerchr added a commit that referenced this pull request Jul 1, 2022
* upstream/main:
  fix merge conflict
  Squashed 'buildres/csl/csl-locales/' changes from 969d9567ac..55459cd79f
  Squashed 'buildres/csl/csl-styles/' changes from e740261..3d3573c
  Show pdf icon for mime type in maintable (#8935)
  Fix for Dark theme not readable (#8929)
  Find Unlinked files should ignore Thumbs.db, etc (#8800)
  Try to  make reproducible builds (#8925)
  Link to new board (and refine text)
  Pass the data as a string (#8923)
  Add IntelliJ plugins to Gitpod configuration
  Add gradle support
  Fix extension name
  Remove obsolete closing bracket
Siedlerchr added a commit to LIM0000/jabref that referenced this pull request Jul 9, 2022
* upstream/main:
  Disable ResearchGateTest on CI (JabRef#8955)
  Bump checkstyle from 10.1 to 10.3.1 (JabRef#8950)
  Bump checkstyle from 10.1 to 10.3.1 (JabRef#8950)
  Bump mariadb-java-client from 2.7.5 to 2.7.6 (JabRef#8953)
  Add notification when adding previous entries to new group configuration (JabRef#8919)
  Remember Sidepane width after restart (JabRef#8936)
  move "Warn about duplicates on import" preferences option (JabRef#8937)
  Fix theme switching exception (JabRef#8939)
  fix merge conflict
  Squashed 'buildres/csl/csl-locales/' changes from 969d9567ac..55459cd79f
  Squashed 'buildres/csl/csl-styles/' changes from e740261..3d3573c
  Show pdf icon for mime type in maintable (JabRef#8935)
  Fix for Dark theme not readable (JabRef#8929)
  Find Unlinked files should ignore Thumbs.db, etc (JabRef#8800)
  Try to  make reproducible builds (JabRef#8925)
  Link to new board (and refine text)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Find unlinked files should ignore Thumbs.db etc.
6 participants