JabRef / jabref

Graphical Java application for managing BibTeX and biblatex (.bib) databases
https://devdocs.jabref.org
MIT License
3.63k stars 2.59k forks source link

Warning Message for Moving Attached Open Files #10121

Closed Hosein-Rahnama closed 2 weeks ago

Hosein-Rahnama commented 1 year ago

Suppose that you attach a file to an entry. Then, using a cleanup you want to move that file to the General File Directory of the library. If the attached file is open, the file is not moved but no warning message is shown. It would be nice to have such a message to let the user know what just happened.

ThiloteE commented 1 year ago

Agreed

DohaRamadan commented 1 year ago

Can i work on this?

ThiloteE commented 1 year ago

Sure, go ahead :-)

koppor commented 7 months ago
Simran-Sunil commented 5 months ago

Can I work on this issue?

github-actions[bot] commented 5 months ago

Welcome to the vibrant world of open-source development with JabRef!

Newcomers, we're excited to have you on board. Start by exploring our Contributing guidelines, and don't forget to check out our workspace setup guidelines to get started smoothly.

Having any questions or issues? Feel free to ask here on GitHub. Need help setting up your local workspace? Join the conversation on JabRef's Gitter chat. And don't hesitate to open a (draft) pull request early on to show the direction it is heading towards. This way, you will receive valuable feedback.

Happy coding! 🚀

HrithikPadavala commented 5 months ago

I want to grab this issue if it is still open and no one is working on it

koppor commented 5 months ago

@Simran-Sunil I didn't see any activity for three weeks, thus, I assigned @HrithikPadavala .

HrithikPadavala commented 4 months ago

Hi @koppor , I have spotted the corresponding function and added log warning as you can see in the attached picture but unable to instantiate DialogServive/JabRefDialogService variable inside MoveFileCleanup.java to call notify method. Could you please tell me how can I create and call notify method

Untitled

HrithikPadavala commented 4 months ago

Should I call notify from within CleanupAction.java only??

koppor commented 4 months ago

Please use the common logging practices:

- LOGGER. warn (exception. toString());
+ LOGGER. warn ("Could not move file", exception);
koppor commented 4 months ago

@HrithikPadavala Use org.jabref.gui.DialogService#notify. Pass Localization.lang("Could not move file.") as parameter. If dialogService is not available, get it passed via the constructor.

HrithikPadavala commented 4 months ago

Hi @koppor , solved the issue and opened a PR

koppor commented 4 months ago

Hi @koppor , solved the issue and opened a PR

@HrithikPadavala The PR doesn't seem to link the this issue (as required in the PR description template). Otherwise, I would see a link to it here.

I am on my mobile and cannot check the PR list....

HrithikPadavala commented 4 months ago

Updated everything as required and committed too. Someone please check and confirm. Thanks.

koppor commented 4 months ago

Updated everything as required and committed too. Someone please check and confirm. Thanks.

Please look at the pull request. The automatic checks show errors:

image

See https://github.com/JabRef/jabref/pull/11438. Scroll down. Click on "details" at each failing test.

After you fixed them, we can have a detailed look. Otherwise our brains are too distracted by issues obvious to us.

HrithikPadavala commented 4 months ago

Opened another PR with branch other than main this time to resolve merge issues please review

koppor commented 4 months ago

Opened another PR with branch other than main this time to resolve merge issues please review

Please resolve the merge conflicts so that the CI can run. First, all checkstyle issues should be solved. This enables us to focus on the content.

HrithikPadavala commented 4 months ago

@koppor Resolved all issues but something related to changelog is not getting resolved. This is the only file need to be reviewed. So that this issue can be closed.

koppor commented 4 months ago

@koppor Resolved all issues but something related to changelog is not getting resolved. This is the only file need to be reviewed. So that this issue can be closed.

Please try to keep context in comments. Comments on PR issues should go to PRs. I replied at https://github.com/JabRef/jabref/pull/11484#issuecomment-2223871387.

juliusalberto commented 1 month ago

Hi, can I work on this issue? Thanks!

ThiloteE commented 1 month ago

Sure, go ahead.

github-actions[bot] commented 1 month ago

Welcome to the vibrant world of open-source development with JabRef!

Newcomers, we're excited to have you on board. Start by exploring our Contributing guidelines, and don't forget to check out our workspace setup guidelines to get started smoothly.

In case you encounter failing tests during development, please check our developer FAQs!

Having any questions or issues? Feel free to ask here on GitHub. Need help setting up your local workspace? Join the conversation on JabRef's Gitter chat. And don't hesitate to open a (draft) pull request early on to show the direction it is heading towards. This way, you will receive valuable feedback.

Happy coding! 🚀

juliusalberto commented 1 month ago

Hi - it seems like that I cannot recreate the issue in my environment. Is it something to do with how Mac filesystem works? When I tried to cleanup things - even though the file is open somewhere else (e.g. Microsoft Word), it won't throw any IOException, it just moves it effortlessly.

Do you think I should try it with a Windows machine? Thanks

Siedlerchr commented 1 month ago

Linux and Mac have different file systems and no atomic lock on the file and if I remember correctly it's Windows only. So you should test on Windows

koppor commented 1 month ago

Do you think I should try it with a Windows machine? Thanks

You can "quickly" fire up a Windows 10 VM using Vagrant as described at https://github.com/JabRef/jabref/tree/main/scripts/vms. https://github.com/JabRef/jabref/tree/main/scripts/vms/windows10 proived the Vagrantfile for Windows 10.