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

Fix split multiline localization #9814

Merged
merged 3 commits into from
May 2, 2023
Merged

Conversation

koppor
Copy link
Member

@koppor koppor commented Apr 28, 2023

While checking #9811, I found a very strange translation key. I fixed that (using our \n possibility) and made the string consistent to an existing one.

Also removed a space at the end of a localization key. Not sure whether I should create a test case for that?

I also ignored a file created by the tool https://structure101.com/.

Compulsory checks

@Siedlerchr
Copy link
Member

Siedlerchr commented Apr 29, 2023 via email

@koppor koppor mentioned this pull request Apr 29, 2023
@koppor
Copy link
Member Author

koppor commented Apr 29, 2023

I only changed the localization. Maybe @HoussemNasri - do you have an idea how to trigger that? The string was introduced at #9625 (where @Siedlerchr was more relaxed regarding text cases ^^).

@Siedlerchr
Copy link
Member

Create a new folder, put that in main file directory and delete the folder again? then go to prefs again?

@HoussemNasri
Copy link
Member

Or you could change the main file directory to a folder that doesn't exist

Comment on lines -57 to -66
ValidationMessage error = ValidationMessage.error(String.format(
"%s%n%s%n%n%s%n%n%s > %s > %s",
Localization.lang("Main directory not found"),
mainDirectoryPath,
Localization.lang("Please select a valid main directory under"),
Localization.lang("Linked files"),
Localization.lang("File directory"),
Localization.lang("Main file directory")
));

Copy link
Member

Choose a reason for hiding this comment

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

Why the hell did I do this...

Copy link
Member Author

Choose a reason for hiding this comment

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

Re-use of existing translations I guess.

Good idea though. I try to re-use translations by grouping similar translations next to each other. Then, at the review on GitHub (by @calixtus e.g.), it can come up. Other than that, I hope that Crowdin also uses sub strings for its "brain" suggestions.

Copy link
Member

Choose a reason for hiding this comment

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

Idea was to have just one common warning message and the already existing translations of the titles of the tabs and sections as breadcrumbs to find the preference setting.

Copy link
Member

Choose a reason for hiding this comment

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

Otherwise you will have to create new translations for every validation message.

Copy link
Member Author

Choose a reason for hiding this comment

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

Idea was to have just one common warning message

I think, the old message was misleading, because one cannot touch the library preferences.

and the already existing translations of the titles of the tabs and sections as breadcrumbs to find the preference setting.

There (IMHO) already was another message with "Linked files". I adapted that to be consistent.

Finally, I think, the preference is easy to find (we also have the search functionality). Thus, I like the short pointer to the preference.

@HoussemNasri
Copy link
Member

The error says to look at Library properties or linked files preferences but there seems to be some inconsistency between the two

image
image

@Siedlerchr
Copy link
Member

@HoussemNasri No, actually this is correct. The main file dir is across all libraries (global), the other ones are per bib file
https://github.com/JabRef/jabref/blob/bf6e57c8fff19ce70885fdd759ecd0fcd9b7884e/src/main/java/org/jabref/model/database/BibDatabaseContext.java#L135-L155C3

@HoussemNasri
Copy link
Member

I see, then the error message needs to be changed because configuring file directories in Library properties won't prevent the error

@koppor
Copy link
Member Author

koppor commented Apr 29, 2023

Side note: We try to explain the different directories at https://docs.jabref.org/setup/databaseproperties#override-default-file-directories.

@koppor
Copy link
Member Author

koppor commented Apr 29, 2023

I see, then the error message needs to be changed because configuring file directories in Library properties won't prevent the error

I don't get that part. The validation message points to the global preferences and not to the settings in the library. Then, changing the global preferences should fix the error?

@HoussemNasri
Copy link
Member

Yes, only changing the global preference will fix the error. I just checked, I set a valid path in Library properties and an invalid main file directory in preferences, but I still couldn't save the preferences and got an error dialog. I don't know much about the directory override mechanism, but it isn't triggered when saving preferences.

image

@koppor
Copy link
Member Author

koppor commented Apr 30, 2023

I don't know much about the directory override mechanism,

Usage example:

Optional<Path> dir = databaseContext.getFirstExistingFileDir(preferences.getFilePreferences());

but it isn't triggered when saving preferences.

And it cannot be triggered there, because the global preferences should be independent of the current library.

Yes, only changing the global preference will fix the error. I just checked, I set a valid path in Library properties and an invalid main file directory in preferences, but I still couldn't save the preferences and got an error dialog.

I fixed the string (and adapted the others)

-                            Localization.lang("Main file directory %0 not found.\nCheck the preferences (linked files) or the library properties.", mainDirectoryPath)
+                            Localization.lang("Main file directory '%0' not found.\nCheck the tab \"Linked files\".", mainDirectoryPath)

@koppor koppor added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Apr 30, 2023
@ThiloteE ThiloteE added the type: code-quality Issues related to code or architecture decisions label May 1, 2023
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.

From my point of view, it's now clearer

@koppor koppor merged commit 5bc359f into JabRef:main May 2, 2023
@koppor koppor deleted the fix-translations branch May 2, 2023 07:30
Siedlerchr added a commit that referenced this pull request May 3, 2023
* upstream/main: (72 commits)
  Fix split multiline localization (#9814)
  New Crowdin updates (#9834)
  change versin to 0.8.10
  remove  jacoco version config
  Bump org.junit.platform:junit-platform-launcher from 1.9.2 to 1.9.3
  Bump org.jsoup:jsoup from 1.15.4 to 1.16.1
  Bump org.junit.jupiter:junit-jupiter from 5.9.2 to 5.9.3
  Bump com.puppycrawl.tools:checkstyle from 10.9.3 to 10.10.0
  Bump com.vladsch.flexmark:flexmark from 0.64.0 to 0.64.2
  Refresh example styles
  Squashed 'buildres/csl/csl-styles/' changes from 4728902faa..a985762505
  New translations JabRef_en.properties (Tagalog)
  New translations JabRef_en.properties (Persian)
  New translations JabRef_en.properties (Indonesian)
  New translations JabRef_en.properties (Portuguese, Brazilian)
  New translations JabRef_en.properties (Vietnamese)
  New translations JabRef_en.properties (Chinese Traditional)
  New translations JabRef_en.properties (Chinese Simplified)
  New translations JabRef_en.properties (Ukrainian)
  New translations JabRef_en.properties (Turkish)
  ...
Siedlerchr added a commit that referenced this pull request May 4, 2023
* upstream/main:
  Fix modernizer (#9824)
  Improve search history by attaching change listener (#9794)
  Fix split multiline localization (#9814)
  New Crowdin updates (#9834)
  change versin to 0.8.10
  remove  jacoco version config
Siedlerchr added a commit to brunaoo/jabref that referenced this pull request May 5, 2023
* upstream/main: (88 commits)
  Minimal config for openRewrite
  Fix missing #
  Fix modernizer (JabRef#9824)
  CHANGELOG.md
  Removed unused code
  Refined ui
  Dissolved FileTab and moved contents to EntryTab
  Renamed CustomEditorFieldsTab to EntryEditorTabsTab
  Fixed antipattern, fixed radiobutton with checkbox
  Renamed ImportExportPreferences to ExportPreferences
  Improve search history by attaching change listener (JabRef#9794)
  Separated WebSearchPrefs and ExportPrefs
  Separated WebSearchTab and ExportTab
  Renamed ImportExportTab to WebSearchTab
  Fix split multiline localization (JabRef#9814)
  New Crowdin updates (JabRef#9834)
  change versin to 0.8.10
  remove  jacoco version config
  Bump org.junit.platform:junit-platform-launcher from 1.9.2 to 1.9.3
  Bump org.jsoup:jsoup from 1.15.4 to 1.16.1
  ...
Siedlerchr added a commit to GuyPuts/jabref that referenced this pull request May 6, 2023
…biblatex-date-formats

* upstream/main: (132 commits)
  Add four rules not having any effect
  Apply BooleanChecksNotInverted
  Remove unused RadioButtonCell
  Minimal config for openRewrite
  Fix missing #
  Fix modernizer (JabRef#9824)
  CHANGELOG.md
  Removed unused code
  Refined ui
  Dissolved FileTab and moved contents to EntryTab
  Renamed CustomEditorFieldsTab to EntryEditorTabsTab
  Fixed antipattern, fixed radiobutton with checkbox
  Renamed ImportExportPreferences to ExportPreferences
  Improve search history by attaching change listener (JabRef#9794)
  Separated WebSearchPrefs and ExportPrefs
  Separated WebSearchTab and ExportTab
  Renamed ImportExportTab to WebSearchTab
  Fix split multiline localization (JabRef#9814)
  New Crowdin updates (JabRef#9834)
  change versin to 0.8.10
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers type: code-quality Issues related to code or architecture decisions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants