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

Fixes problems with managing external file types (issue 5846) #5894

Merged
merged 3 commits into from
Feb 2, 2020

Conversation

systemoperator
Copy link
Contributor

@systemoperator systemoperator commented Jan 29, 2020

Fixes #5846

Changed:

  • edit/customize external file types: radio button for "default program"/"custom program" fixed: They now get updated and set properly (binding added and setting values).
  • "Edit file type" dialog: correct value gets loaded now for the input field "Name"
  • when updating an external file type entry: no differentiation will be made anymore whether the program runs on Windows or somewhere else
  • Everything concerning the type CustomExternalFileType is now working properly as well.
    • Settings will also be saved to JabRef's config.xml file.
  • Editing an entry with type ExternalFileType works now as well. (changes are not lost anymore)
    • Settings will also be saved to JabRef's config.xml file.

Remarks:

  • Table in "Manage external file types" dialog is "lazily" updated when a table entry has been updated. (But this seems to be a Linux issue anyway, since other tables behave the same way.)

Tasks:

  • Change in CHANGELOG.md described
  • Manually tested changed features in running JabRef (always required)
  • Checked documentation: Is the information available and up to date?

@systemoperator systemoperator changed the title [WIP] Addresses problems with managing external file types (issue 5846) Addresses problems with managing external file types (issue 5846) Jan 29, 2020
@systemoperator systemoperator changed the title Addresses problems with managing external file types (issue 5846) Fixes problems with managing external file types (issue 5846) Jan 29, 2020
@systemoperator
Copy link
Contributor Author

systemoperator commented Jan 29, 2020

The last commit is a quick fix and could be improved, since the old table entry gets replaced by a new one, even if the user would then cancel the process of editing the entry. Nevertheless, even if the user cancelled the edit process, everything is still fine and valid.

Editing external file types is now fully functional. No problems are known. Suggestions are welcome.

The line fileTypesTable.refresh(); has no effect, since the "lazy" update of the table does not change by using it. But this could be a Linux issue anyway, since other tables behave the same way. Thus, I could remove the line.

@calixtus
Copy link
Member

Thanks for the fix @systemoperator.
I took a quick peek into the code, there seems to be something wrong with the branch head. Git wants to merge a lot of additions to the csl-styles. I'm not really into git, but maybe a rebase on the current upstream/master can clear that. Just a wild guess.

@systemoperator
Copy link
Contributor Author

systemoperator commented Jan 29, 2020

Yes, I have noticed this as well. github actions "did" that. It's the first time, that I have seen that, but I am new here anyway, so I don't know whether this occurs sometimes and it is fine or not. The good thing is, that there are no merge conflicts. Maybe @tobiasdiez can have a look at this.

@calixtus
Copy link
Member

Hey @systemoperator, got a hint by @koppor. This might help solving the branch issue with your PR:

https://lostechies.com/joshuaflanagan/2010/09/03/use-gitk-to-understand-git/

@koppor
Copy link
Member

koppor commented Jan 30, 2020

You can create one single commit containing your changes with following commands:

  1. Ensure that your workdirectory is clean (git shows now changes)
  2. gitk --all -- just keep it opened until the end. Then, you can always revert to a state you did some reset steps.
  3. git checkout fix-for-issue-5846
  4. git fetch --all
  5. git merge upstream/master (and commit the merge!)
  6. git reset upstream/master
  7. Commit as usual
  8. git push -f

Assumption: git remote add upstream https://github.com/JabRef/jabref.git was done sometime ago. You can check with git remote -v.

…am"/"custom program" fixed: it now gets updated properly (binding added);

- "Edit file type dialog": correct value get loaded now for the input field "Name"
- when storing an updated file type: no differentiation will be made any more whether program runs on Windows or somewhere else
- quick fix for pending issue: editing an ExternalFileType works now (still room for improvement); visual bug concerning "lazy" update of table could be a Linux issue, since other tables are affected as well; so basically - except the "lazy" update thing -- everything works concerning external file types
Copy link
Member

@tobiasdiez tobiasdiez 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 PR. The changes look good to me. I've one remark concerning the introduction of the custom application property, which I don't think is necessary.

@@ -40,6 +40,7 @@ public CustomizeExternalFileTypesDialog() {
this.setResultConverter(button -> {
if (button == ButtonType.OK) {
viewModel.storeSettings();
fileTypesTable.refresh();
Copy link
Member

Choose a reason for hiding this comment

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

If it does not work, then you can delete this indeed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@systemoperator
Copy link
Contributor Author

For testing purposes, I made some changes in EditExternalFileTypeEntryDialog.fxml (in the src/main.../gui/... directory) but when I built and ran the project with the changes, then those changes did not show up in the corresponding dialogue.

My questions are:

  • What needs to be done, so that changes in those files will update the corresponding GUI element?
  • Do you use some graphical tool to change these files?

@calixtus
Copy link
Member

calixtus commented Feb 1, 2020

Are the multiple changes to authors.md really intended?

@systemoperator
Copy link
Contributor Author

systemoperator commented Feb 1, 2020

Yes, I simply ran the script scripts/generate-authors.sh, to pull in new authors.

@calixtus
Copy link
Member

calixtus commented Feb 1, 2020

Theres something odd: Igor Steinmacher appears in full name as well in concatenaded form (also Oscar Gustafsson ) Maybe you've configured git to return only nicknames instead of full names?

@Siedlerchr
Copy link
Member

You don't need to run the script manually. We will do this from time to time or at least in preparation for a release

@systemoperator
Copy link
Contributor Author

Theres something odd: Igor Steinmacher appears in full name as well in concatenaded form (also Oscar Gustafsson )

Maybe he commited with his username first and later on with his real name. Nevertheless, this will always reappear by running the script. But I could now remove the entry with his username?

I did it, because it is mentioned in the docs, that one can do it. And I wanted to try it. :)

@tobiasdiez
Copy link
Member

@koppor what is our policy concerning this? (which then should be clarified in the docs)

As these changes are not directly related to the main theme of this PR, can you please remove these changes for now so that we can merge? Thanks

@koppor
Copy link
Member

koppor commented Feb 1, 2020

We maintain the .mailmap to always map to the full name. I don't get, why is Oscar and Igor appear with their short names at your system. Have to try it here locally

Typically, we update the AUTHORS file now and then. Especially before a release. (See https://github.com/JabRef/jabref/wiki/Releasing-a-new-Version)

I also thought to setup a GitHub action to comment at the PR with the new entry in the AUTHORS file. Should be doable...

@calixtus
Copy link
Member

calixtus commented Feb 1, 2020

Maybe a quick fix to get this PR mergable: leave your name in authors.md and remove the other additions, they will be put in before release.

@systemoperator
Copy link
Contributor Author

done

@tobiasdiez
Copy link
Member

Thanks again for contribution, and sorry for the confusion caused. I hope the additional effort on your side wasn't too great. Your next PR should go more smoothly 😄

@tobiasdiez tobiasdiez merged commit 78f7d65 into JabRef:master Feb 2, 2020
Siedlerchr added a commit that referenced this pull request Feb 2, 2020
* upstream/master:
  Fixes problems with managing external file types (issue 5846) (#5894)
  Squashed 'src/main/resources/csl-locales/' changes from 41da445acc..4fa753374e
  Squashed 'src/main/resources/csl-styles/' changes from f0c7374..e71363e
Siedlerchr added a commit that referenced this pull request Feb 3, 2020
* upstream/master:
  reference to issue added (#5911)
  Fix properly resolves OrFields of required fields (#5903)
  IDE setup updated and extended (#5901)
  Squashed 'src/main/resources/csl-styles/' changes from e71363e..c531528
  Fixes problems with managing external file types (issue 5846) (#5894)
  Squashed 'src/main/resources/csl-locales/' changes from 41da445acc..4fa753374e
  Squashed 'src/main/resources/csl-styles/' changes from f0c7374..e71363e
  Update development-strategy.md
Siedlerchr added a commit that referenced this pull request Feb 3, 2020
* master:
  reference to issue added (#5911)
  Fix properly resolves OrFields of required fields (#5903)
  IDE setup updated and extended (#5901)
  Squashed 'src/main/resources/csl-styles/' changes from e71363e..c531528
  Fixes problems with managing external file types (issue 5846) (#5894)
  Squashed 'src/main/resources/csl-locales/' changes from 41da445acc..4fa753374e
  Squashed 'src/main/resources/csl-styles/' changes from f0c7374..e71363e
  Update development-strategy.md
  Replace link to Workspace set-up with new one (#5896)
  Fixes making paths of linked files relative (web urls will not be touched anymore) (#5879)
koppor pushed a commit that referenced this pull request Feb 15, 2022
eb97405 Create frattura-ed-integrita-strutturale-fracture-and-structural-inte… (#5877)
b33ebfc make journal names title case (#5900)
1c7ecf2 Update uclouvain-centre-charles-de-visscher-pour-le-droit-internation… (#5901)
1a2ea12 Create trinity-college-dublin-zoology-botany-environmental-sciences-h… (#5893)
6bd742c Update harvard-anglia-ruskin-university.csl (#5840)
bc8f258 Update chicago-author-date.csl (#5836)
80aded6 Fix missing prefix for URL field (#5894)
b850a0d Update american-society-of-civil-engineers.csl (#5891)
94c1cb3 Update deutsche-gesellschaft-fur-psychologie.csl (#5861)
f66f384 Update universite-cheikh-anta-diop-faculte-de-medecine-de-pharmacie-et-dodontologie.csl (#5863)
82cf786 Update and rename acta-psychiatrica-scandinavica.csl to dependent/acta-psychiatrica-scandinavica.csl (#5879)

git-subtree-dir: buildres/csl/csl-styles
git-subtree-split: eb97405
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.

Changing settings in "Options" > "Manage external file types" has no effect
5 participants