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

Respect OS language at NativeDesktop#getDefaultFileChooserDirectory #9837

Merged
merged 14 commits into from
May 18, 2023
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- We fixed an issue where it was no longer possible to connect to a shared mysql database due to an exception [#9761](https://github.com/JabRef/jabref/issues/9761)
- We fixed the citation key generation for (`[authors]`, `[authshort]`, `[authorsAlpha]`, `authIniN`, `authEtAl`, `auth.etal`)[https://docs.jabref.org/setup/citationkeypatterns#special-field-markers] to handle `and others` properly. [koppor#626](https://github.com/koppor/jabref/issues/626)
- We fixed the Save/save as file type shows BIBTEX_DB instead of "Bibtex library" [#9372](https://github.com/JabRef/jabref/issues/9372)
- We fixed the default main file directory for non-English Linux users. [#8010](https://github.com/JabRef/jabref/issues/8010)

### Removed

Expand Down
24 changes: 9 additions & 15 deletions docs/decisions/0026-use-jna-to-determine-default-directory.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,10 @@ Swing's FileChooser implemented a very decent directory determination algorithm.
It thereby uses `sun.awt.shell.ShellFolder`.

* Good, because provides best results on most platforms.
* Good, because also supports localization of the folder name. E.g., `~/Dokumente` in Germany.
* Bad, because introduces a dependency on Swing and thereby contradicts the second decision driver.
* Bad, because GraalVM's support Swing is experimental
* Bad, because GraalVM's support Swing is experimental.
* Bad, because handles localization only on Windows.

### Use `user.home`

Expand All @@ -44,25 +46,17 @@ There is `System.getProperty("user.home");`.
* Bad, because "The concept of a HOME directory seems to be a bit vague when it comes to Windows". See <https://stackoverflow.com/a/586917/873282> for details.
* Bad, because it does not include `Documents`:
As of 2022, `System.getProperty("user.home")` returns `c:\Users\USERNAME` on Windows 10, whereas
`FileSystemView` returns `C:\Users\USERNAME\Documents`, which is the "better" directory
`FileSystemView` returns `C:\Users\USERNAME\Documents`, which is the "better" directory.

### AppDirs

> AppDirs is a small java library which provides a path to the platform dependent special folder/directory.

* Good, because already used in JabRef
* Bad, because does not use `Documents` on Windows, but rather `C:\Users\<Account>\AppData\<AppAuthor>\<AppName>` as basis
* Good, because already used in JabRef.
* Bad, because does not use `Documents` on Windows, but rather `C:\Users\<Account>\AppData\<AppAuthor>\<AppName>` as basis.

### Java Native Access

* Good, because no additional dependency required, as it is already loaded by AppDirs
* Good, because it is well maintained and widely used
* Good, because it provides direct access to `Documents` and other system variables

## More Information

{You might want to provide additional evidence/confidence for the decision outcome here and/or
document the team agreement on the decision and/or
define when this decision when and how the decision should be realized and if/when it should be re-visited and/or
how the decision is validated.
Links to other decisions and resources might here appear as well.}
* Good, because no additional dependency required, as it is already loaded by AppDirs.
* Good, because it is well maintained and widely used.
* Good, because it provides direct access to `Documents` and other system variables.
28 changes: 13 additions & 15 deletions src/main/java/org/jabref/gui/desktop/JabRefDesktop.java
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,9 @@ public static void openExternalViewer(BibDatabaseContext databaseContext,
} else if (StandardField.EPRINT.equals(field)) {
IdentifierParser identifierParser = new IdentifierParser(entry);
link = identifierParser.parse(StandardField.EPRINT)
.flatMap(Identifier::getExternalURI)
.map(URI::toASCIIString)
.orElse(link);
.flatMap(Identifier::getExternalURI)
.map(URI::toASCIIString)
.orElse(link);

if (Objects.equals(link, initialLink)) {
Optional<String> eprintTypeOpt = entry.getField(StandardField.EPRINTTYPE);
Expand Down Expand Up @@ -130,15 +130,15 @@ private static void openDoi(String doi) throws IOException {
}

public static void openCustomDoi(String link, PreferencesService preferences, DialogService dialogService) {
DOI.parse(link)
.flatMap(doi -> doi.getExternalURIWithCustomBase(preferences.getDOIPreferences().getDefaultBaseURI()))
.ifPresent(uri -> {
try {
JabRefDesktop.openBrowser(uri);
} catch (IOException e) {
dialogService.showErrorDialogAndWait(Localization.lang("Unable to open link."), e);
}
});
DOI.parse(link)
.flatMap(doi -> doi.getExternalURIWithCustomBase(preferences.getDOIPreferences().getDefaultBaseURI()))
.ifPresent(uri -> {
try {
JabRefDesktop.openBrowser(uri);
} catch (IOException e) {
dialogService.showErrorDialogAndWait(Localization.lang("Unable to open link."), e);
}
});
}

/**
Expand Down Expand Up @@ -263,7 +263,6 @@ public static void openBrowserShowPopup(String url, DialogService dialogService)
* If no command is specified in {@link Globals}, the default system console will be executed.
*
* @param file Location the console should be opened at.
* @param dialogService
*/
public static void openConsole(File file, PreferencesService preferencesService, DialogService dialogService) throws IOException {
if (file == null) {
Expand Down Expand Up @@ -292,8 +291,7 @@ public static void openConsole(File file, PreferencesService preferencesService,
new ProcessBuilder(subcommands).start();
} catch (IOException exception) {
LOGGER.error("Open console", exception);

dialogService.notify(Localization.lang("Error occured while executing the command \"%0\".", command));
dialogService.notify(Localization.lang("Error occured while executing the command \"%0\".", command));
}
}
}
Expand Down
5 changes: 0 additions & 5 deletions src/main/java/org/jabref/gui/desktop/os/DefaultDesktop.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,4 @@ public String detectProgramPath(String programName, String directoryName) {
public Path getApplicationDirectory() {
return getUserDirectory();
}

@Override
public Path getDefaultFileChooserDirectory() {
return Path.of(System.getProperty("user.home"));
}
}
27 changes: 22 additions & 5 deletions src/main/java/org/jabref/gui/desktop/os/Linux.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
import java.io.File;
import java.io.IOException;
import java.io.InputStreamReader;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
import java.util.Locale;
import java.util.Objects;
import java.util.Optional;

import org.jabref.architecture.AllowedToUseAwt;
Expand All @@ -20,6 +21,7 @@
import org.jabref.gui.util.StreamGobbler;
import org.jabref.logic.l10n.Localization;

import com.microsoft.applicationinsights.core.dependencies.apachecommons.io.IOUtils;
koppor marked this conversation as resolved.
Show resolved Hide resolved
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -171,9 +173,24 @@ public Path getApplicationDirectory() {

@Override
public Path getDefaultFileChooserDirectory() {
return Path.of(Objects.requireNonNullElse(
System.getenv("XDG_DOCUMENTS_DIR"),
System.getProperty("user.home") + "/Documents")
);
String xdgDocumentsDir = System.getenv("XDG_DOCUMENTS_DIR");
if (xdgDocumentsDir != null) {
return Path.of(xdgDocumentsDir);
}

// Make use of xdg-user-dirs
// See https://www.freedesktop.org/wiki/Software/xdg-user-dirs/ for details
try {
Process process = new ProcessBuilder("xdg-user-dirs", "DOCUMENTS").start();

Choose a reason for hiding this comment

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

the script is called xdg-user-dir without the s. Its a mess i know. The ""Toolkit"" is called -dirs but the script that you actually run is called -dir

Copy link

@dermalikmann dermalikmann May 11, 2023

Choose a reason for hiding this comment

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

I tested it anyways by copying the script with the modified name into my path, and it successfully puts /home/<USER>/Dokumente into the "Main file directory" field.

List<String> strings = IOUtils.readLines(process.getInputStream(), StandardCharsets.UTF_8);
String documentsPath = strings.get(0);
koppor marked this conversation as resolved.
Show resolved Hide resolved
LOGGER.debug("Got documents path {}", documentsPath);
return Path.of(documentsPath);
} catch (IOException e) {
LOGGER.error("Error while executing xdg-user-dirs", e);
}

// Fallback
return getUserDirectory();
}
}
10 changes: 9 additions & 1 deletion src/main/java/org/jabref/gui/desktop/os/NativeDesktop.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.jabref.gui.desktop.os;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;

import org.jabref.gui.DialogService;
Expand Down Expand Up @@ -35,7 +36,14 @@ public interface NativeDesktop {
*
* @return The path to the directory
*/
Path getDefaultFileChooserDirectory();
default Path getDefaultFileChooserDirectory() {
Path userDirectory = getUserDirectory();
Path documents = userDirectory.resolve("Documents");
if (!Files.exists(documents)) {
return userDirectory;
}
return documents;
}

/**
* Returns the path to the system's user directory.
Expand Down
5 changes: 0 additions & 5 deletions src/main/java/org/jabref/gui/desktop/os/OSX.java
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,4 @@ public String detectProgramPath(String programName, String directoryName) {
public Path getApplicationDirectory() {
return Path.of("/Applications");
}

@Override
public Path getDefaultFileChooserDirectory() {
return Path.of(System.getProperty("user.home") + "/Documents");
}
}
1 change: 1 addition & 0 deletions src/main/resources/tinylog.properties
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ writerAzure = application insights
exception = strip: jdk.internal

#[email protected] = debug
[email protected] = debug