Add a more descriptive path when Directory cannot be found by devjuliet · Pull Request #7232 · JabRef/jabref

devjuliet · 2020-12-23T05:01:29Z

Issue problem: When you try to download a pdf (Lookup -> Search full documents online ) and you don't have a file directory set, an alert tells you that you should follow a path to set your file directory: Preference -> File. But that path is not correct because to set the file directory you need to follow this path: Options -> Preference -> Linked Files -> Main File Directory

Suggested changes: change the message to: Main file directory not set! Check the Preferences (Linked files) or the Library properties.

Screenshots:

Fixes #7197

Change in CHANGELOG.md described (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 documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

Siedlerchr · 2020-12-23T07:33:10Z

Please see https://docs.jabref.org/setup/databaseproperties#override-default-file-directories
You can define relative directories in the library properties and thus the main file directory can be empty. In addition there's a checkbox to always use the bib file location which will override all other settings.

suggestion

change the message to No file directory set or. Check the preferences (linked files) or the library properties

Siedlerchr · 2020-12-24T12:41:25Z

-                                    + " -> " + Localization.lang("File"));
+                            Localization.lang("Main file directory not set!\n")
+                                    + Localization.lang("Check the Preferences (Linked files)")
+                                    + Localization.lang(" or the Library properties."));


It's better and easier for the translators if you put this in one string. See https://devdocs.jabref.org/getting-into-the-code/code-howtos#using-localization-correctly

We should also take care about casing - maybe less is better?

Main file directory not set. Check the preferences (linked files) or the library properties.

Great! I'll work on it

Changed to one string and also changed the message 😄

Thanks! Now you need to add this to the resources/l10n/en.properties file as well (see the LocalizationConsistencyTest) under l10n.
When the key contains spaces, you need to add a backslash after the word before the space. (see the other entries in the file)

Now the test fail complaining that a newline is illegal - see: https://github.com/JabRef/jabref/pull/7232/checks?check_run_id=1618423949#step:6:122

java.lang.RuntimeException: Main file directory not set.nCheck the preferences (linked files) or the library properties. contains a new line character. As this is a localization key, this is illegal!

You can try with   or  in the localization - such as other localizations did. Could you please try and report back?

BTW: Hint: How to read error outputs.

See the red crosses

Experience in JabRef tells: Click at Details at "Tests / Unit Tests"

Experience tells to scroll up, until one sees a red "Error"

Then try to understand the error message. I tried to explain the error message at my last commit. My wish, however, is that I am not the only one knowing how to click on "Details", scroll up and find "Error"

You can try with
or
in the localization - such as other localizations did. Could you please try and report back?

javafx no longer uses html, so \n is correct

@Siedlerchr May I ask whether you've seent he screen shot error output? Here the deep link to the method throwing the exception:

jabref/src/test/java/org/jabref/logic/l10n/LocalizationParser.java

Line 287 in afdb194

throw new RuntimeException(languageKey + " contains a new line character. As this is a localization key, this is illegal!");

It throws an exception if \n is contained there. Thus, we cannot use \n. Since you say that  and   cannot be used, we cannot have any newlines any more. Am I right?

devjuliet · 2020-12-29T19:58:21Z

Thanks for the review! I added some changes 😀

Siedlerchr

Thanks, looks good to me now!

calixtus · 2021-01-11T18:44:43Z

Huh, I think we totally forgot your PR when thinking about the issue with #7279. But since your PR is now independent of this issue, I don't see any cause not to merge, since every other remark is implemented.
Sorry for the delay and thanks for your work!

…dtask * upstream/master: Update guidelines-for-setting-up-a-local-workspace.md (#7339) Updates to colored group indicator for cited entries (#7173) Add some special fields as default columns (#7286) Add a more descriptive path when Directory cannot be found (#7232) Bump antlr4 from 4.9 to 4.9.1 (#7327) Bump unirest-java from 3.11.09 to 3.11.10 (#7329) Bump mockito-core from 3.6.28 to 3.7.0 (#7328) Bump antlr4-runtime from 4.9 to 4.9.1 (#7330) Bump gittools/actions from v0.9.7 to v0.9.8 (#7331) Update to gradle 6.8 (#7324) Link to GitHub contributors in about dialog (#7319) Fix snapcraft upload (#7263)

added more descriptive path when Directory not found

ae109f9

change the message of propmt to: Main file directory not set

8aa35ba

devjuliet changed the title ~~added more descriptive path when Directory not found~~ Add a more descriptive path when Directory not found Dec 24, 2020

Changed message propmt

e56896f

devjuliet marked this pull request as ready for review December 24, 2020 03:01

Siedlerchr reviewed Dec 24, 2020

View reviewed changes

Message in prompt changed to one string

2038ec6

Siedlerchr added the status: changes-required Pull requests that are not yet complete label Dec 29, 2020

devjuliet added 2 commits December 29, 2020 12:45

Deleted new line break in message propmpt due to error

6360888

Added message in JabRef_en.properties

22e12b6

tobiasdiez reviewed Dec 29, 2020

View reviewed changes

Comment thread src/main/java/org/jabref/gui/externalfiles/DownloadFullTextAction.java Outdated

Added header to error propmt dialog

fbb4133

Siedlerchr approved these changes Dec 31, 2020

View reviewed changes

Siedlerchr reviewed Dec 31, 2020

View reviewed changes

Comment thread src/main/java/org/jabref/gui/externalfiles/DownloadFullTextAction.java Outdated

koppor mentioned this pull request Dec 31, 2020

Replace in localization by \n #7279

Merged

5 tasks

deleted extra brace

a74c4cb

calixtus approved these changes Jan 11, 2021

View reviewed changes

calixtus changed the title ~~Add a more descriptive path when Directory not found~~ Add a more descriptive path when Directory cannot be found Jan 11, 2021

calixtus merged commit 54f27e9 into JabRef:master Jan 11, 2021

Uh oh!

Conversation

devjuliet commented Dec 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Siedlerchr commented Dec 23, 2020

suggestion

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devjuliet commented Dec 29, 2020

Uh oh!

Uh oh!

Siedlerchr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

calixtus commented Jan 11, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

devjuliet commented Dec 23, 2020 •

edited

Loading