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

feat: support Unix-like OSes #1035

Merged

Conversation

trombik
Copy link
Contributor

@trombik trombik commented May 22, 2024

As discussed in 1746, here is the patch.

In general, Platform.isUnix() is preferred, which will not break Linux support.

isX11() was suggested. I did search in the code and the web. (Unix|UNIX) is used in several places in the code and the documentation. It looks to me that UNIX(R) is used when referring to the registered trademark, and Unix when referring to Unix-like and Unix variants. Thus, isUnix() is used in the patch.

Pull request type

  • Feature enhancement -> [enhancement]

Which ticket is resolved?

What does this PR change?

  • Support *BSD variants.

Other information

The build was successful, and I am using it. xdg-open is working.

In general, `Platform.isUnix()` is preferred, which will not break Linux
support.
@@ -87,6 +92,8 @@ public enum OsType {
isWindows = osName.startsWith("windows");
isMacOS = osName.startsWith("mac");
isLinux = osName.startsWith("linux");
isBSD = osName.startsWith("bsd");
isUnix = isLinux || isBSD;
Copy link
Member

Choose a reason for hiding this comment

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

isUnixLike is better abstraction. see https://github.com/search?q=isUnixLike&type=code

Suggested change
isUnix = isLinux || isBSD;
isUnixLike = isLinux || isBSD;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should I git push --force the fix, or simply commit another one with the fix?

Copy link
Member

Choose a reason for hiding this comment

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

It is ok to simply commit another one. I will squash and merge when all is ok.

Comment on lines 166 to 167
public static boolean isUnix() {
return isUnix;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public static boolean isUnix() {
return isUnix;
public static boolean isUnixLike() {
return isUnixLike;

@@ -107,7 +114,7 @@ public enum OsType {
isJava_17_orLater = (javaVersion >= toVersion(17, 0, 0, 0));

// UI toolkits
isKDE = (isLinux && System.getenv("KDE_FULL_SESSION") != null);
isKDE = (isUnix && System.getenv("KDE_FULL_SESSION") != null);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
isKDE = (isUnix && System.getenv("KDE_FULL_SESSION") != null);
isKDE = (isUnixLike && System.getenv("KDE_FULL_SESSION") != null);

@@ -61,6 +64,8 @@ public enum OsType {
public static final boolean isWindows;
public static final boolean isMacOS;
public static final boolean isLinux;
public static final boolean isBSD;
public static final boolean isUnix;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public static final boolean isUnix;
public static final boolean isUnixLike;

@@ -312,7 +312,7 @@ public static String getConfigDir() {
}
// Check for UNIX varieties
// Solaris is generally detected as SunOS
} else if (Platform.isLinux()) {
} else if (Platform.isUnix()) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
} else if (Platform.isUnix()) {
} else if (Platform.isUnixLike()) {

@trombik
Copy link
Contributor Author

trombik commented May 23, 2024

built and tested on my machine.

@miurahr miurahr merged commit d27f51f into omegat-org:master May 24, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants