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
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- We fixed an issue where an exception was thrown for the user after <kbd>Ctrl</kbd>+<kbd>Z</kbd> command. [#9737](https://github.com/JabRef/jabref/issues/9737)
- 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 an issue when overwriting the owner was disabled. [#9896](https://github.com/JabRef/jabref/pull/9896)
- We fixed the default main file directory for non-English Linux users. [#8010](https://github.com/JabRef/jabref/issues/8010)
- We fixed an issue regarding recording redundant prefixes in search history. [#9685](https://github.com/JabRef/jabref/issues/9685)
- We fixed an issue where passing a URL containing a DOI led to a "No entry found" notification. [#9821](https://github.com/JabRef/jabref/issues/9821)
- We fixed some minor visual inconsistencies and issues in the preferences dialog. [#9866](https://github.com/JabRef/jabref/pull/9866)
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.
24 changes: 12 additions & 12 deletions src/main/java/org/jabref/gui/desktop/JabRefDesktop.java
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,9 @@ public static void openExternalViewer(BibDatabaseContext databaseContext,
} else if (StandardField.EPRINT == 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 @@ -127,15 +127,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
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 @@ -50,9 +50,4 @@ public String detectProgramPath(String programName, String directoryName) {
public Path getApplicationDirectory() {
return getUserDirectory();
}

@Override
public Path getDefaultFileChooserDirectory() {
return Path.of(System.getProperty("user.home"));
}
}
36 changes: 31 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 Down Expand Up @@ -175,9 +176,34 @@ 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-dir", "DOCUMENTS").start(); // Package name with 's', command without
List<String> strings = new BufferedReader(new InputStreamReader(process.getInputStream(), StandardCharsets.UTF_8))
.lines().toList();
if (strings.isEmpty()) {
LoggerFactory.getLogger(Linux.class).error("xdg-user-dir returned nothing");
return getUserDirectory();
}
String documentsDirectory = strings.get(0);
Path documentsPath = Path.of(documentsDirectory);
if (!Files.exists(documentsPath)) {
LoggerFactory.getLogger(Linux.class).error("xdg-user-dir returned non-existant directory {}", documentsDirectory);
return getUserDirectory();
}
LoggerFactory.getLogger(Linux.class).debug("Got documents path {}", documentsPath);
return documentsPath;
} catch (IOException e) {
LoggerFactory.getLogger(Linux.class).error("Error while executing xdg-user-dir", 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
Expand Up @@ -2,6 +2,7 @@

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

import org.jabref.gui.DialogService;
Expand Down Expand Up @@ -41,7 +42,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 @@ -59,9 +59,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");
}
}