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

Issue #4269 & #3874 #5110

Closed
wants to merge 1 commit into from
Closed

Conversation

Fminorelli
Copy link


  • Change in CHANGELOG.md described
  • Tests created for changes
  • Manually tested changed features in running JabRef
  • Screenshots added in PR description (for bigger UI changes)
  • Ensured that the git commit message is a good one
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)

@Fminorelli Fminorelli changed the title Resolução das issues 4269 e 3874 Issue #4269 & #3874 Jul 9, 2019
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.

Thanks for your contribution. It seems like you based your code on the 4.x branch. However, the 4.x version is no longer maintained. So please base your branch on the latest JabRef master (the upcoming 5.x version)

basePanel.markBaseChanged();
}
});
String str = dir.toString() + entry.getField(FieldName.FILE);
Copy link
Member

Choose a reason for hiding this comment

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

This won't work because dir is an optional. You would need get() first.
Also, File class should be avoided. Better use the path class and the other classes from the nio package:
https://docs.oracle.com/javase/tutorial/essential/io/fileio.html

And your code does not really solve the problem stated in the issue.
The idea is that JabRef previously searched for the file in all directories and linked it, if found.
If not found, it would download the file.

So the idea here would be to first link local files. This is already implemented in the AutoSetFileLinksUtil.
This could be reused or the method called before.

@Siedlerchr
Copy link
Member

Thanks for your contribution, however there seems to be something wrong here with your branch.
Version 4.x is no longer under active development, so please base your changes on the JabRef master branch.

@Fminorelli
Copy link
Author

Fminorelli commented Jul 10, 2019

Thanks for the feedback. I do understand the problems with my resolution of #Issue 3874, and I'll try to make the proper corrections.

But about the #Issue 4269, the problem no longer exists in the development version, eventhough the issue is still open, that's why changes were submited to the previous branch. As a matter of curiosity, is the change made, a proper solution?

@Siedlerchr
Copy link
Member

Hi,
if the issue #4269 is no longer present in the development version than you can add a comment to that issue that it's no longer reproducible and we can close it.
Sometimes issues are resolved or are (automatically/indirectly) fixed as part of another bug fix,someone did, so the issue might be still open, although the bug is fixed.

You can check the code of the current master and see for yourself if the bug was fixed as you intended it.
I have just no code open here, so I can't tell ya.

@Siedlerchr
Copy link
Member

It would be really nice if you could fix this issue for JabRef 5.x!

@Siedlerchr Siedlerchr closed this Aug 18, 2019
github-actions bot pushed a commit that referenced this pull request Dec 1, 2020
a20406d Added name of the editors of a given edition (#5140)
9881fc5 Ping on push, not PR, document role of dist-updater (#5137)
04668cc Create nouvelles-perspectives-en-sciences-sociales.csl (#5063)
1d94e21 Update bursa-uludag-universitesi-saglik-bilimleri-enstitusu.csl (#5047)
84f3893 Add Harvard style for Metropolia University of Applied Sciences (#5086)
8e43e79 Create opto-electronic-advances.csl (#5135)
36e4fba Update society-for-american-archaeology.csl (#5124)
69ca360 St. Paul Canon Law new style (#5138)
b490ab0 Update and rename st-paul-university-faculty-of-canon-law.csl to saint-paul-university-faculty-of-canon-law.csl
b498116 There is no en-CA locale
3c35f28 Metadata
7059cca Create tu-dortmund-agvm.csl (#5088)
c321c98 Create new Citation type (#5093)
a7edc8d Update international-organization.csl (#5103)
3d1a052 The AWS load balancer is messing things up (#5133)
ca3839b Fix sort by a single macro (#5136)
5d1a7e8 Update chungara-revista-de-antropologia-chilena.csl (#5123)
cd75d5d ping distribution-updater (#5132)
dcf473a Update wirtschaftsuniversitat-wien-health-care-management.csl (#5125)
a87085e Fix Harvard Praxisforschung Editors (#5130)
d4176ca Switch automated tests to Github Actions (#5111)
726d0d8 Radiology, MPP, CORR -- small fixes: https://forums.zotero.org/discussion/85883/doi-radiology#latest https://forums.zotero.org/discussion/51058/style-request-molecular-plant-pathology#latest https://forums.zotero.org/discussion/85678/citing-style-clinical-orthopaedics-and-related-research#latest
e23db68 Update to la-trobe-university-harvard style (#5119)
c54b278 Create wirtschaftsuniversitat-wien-health-care-management.csl (#5110)
62fb019 Create austral-entomology.csl (#5118)
afa328c Update iso690-author-date-en.csl (#5113)
5468dce Update iso690-author-date-fr-no-abstract.csl (#5112)
98af86c Update iso690-numeric-fr.csl (#5115)
09f84c4 Update iso690-author-date-fr.csl (#5114)
178a9e4 Fix current biology to superscript
1fa5ce7 Create droit-belge-centre-de-droit-prive-ulb.csl (#5107)
3a6a4bc Fix file modes (#5106)
48f50e5 Create chungara-revista-de-antropologia-chilena.csl (#5096)
1e848f8 Create the-journal-of-the-indian-law-institute.csl (#5100)
856524c Create molecular-biology.csl (#5101)
eeebbf4 Create harvard-harper-adams-university.csl (#5104)
d90993d Fix tests
d4037bf WIP: St Paul Canon Law style

git-subtree-dir: src/main/resources/csl-styles
git-subtree-split: a20406d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants