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.

Sign up for GitHub

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

Jump to bottom

Add support for Kotlin installed by Scoop #486

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add support for Kotlin installed by Scoop #486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion shared/src/main/kotlin/org/javacs/kt/classpath/BackupClassPathResolver.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import java.util.function.BiPredicate
import org.javacs.kt.util.tryResolving
import org.javacs.kt.util.findCommandOnPath
import org.javacs.kt.LOG
import org.javacs.kt.util.OSContext
import java.nio.file.Paths

/** Backup classpath that find Kotlin in the user's Maven/Gradle home or kotlinc's libraries folder. */
Expand Down Expand Up @@ -78,7 +79,7 @@ private fun findKotlinCliCompilerLibrary(name: String): Path? =
// alternative library locations like for snap
// (can probably just use elvis operator and multiple similar expressions for other install directories)
private fun findAlternativeLibraryLocation(name: String): Path? =
Paths.get("/snap/kotlin/current/lib/${name}.jar").existsOrNull()
OSContext.CURRENT_OS.candidateAlternativeLibraryLocations(name).firstNotNullOfOrNull { Paths.get(it).existsOrNull() }

private fun Path.existsOrNull() =
if (Files.exists(this)) this else null
Expand Down
27 changes: 27 additions & 0 deletions shared/src/main/kotlin/org/javacs/kt/util/OSContext.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package org.javacs.kt.util

/**
* Tasks that depends on user's OS
*/
interface OSContext {
/**
* Suggests the candidate locations of the given JAR
*
* @param name the name of the JAR
* @return the candidate full paths to the JAR
*/
fun candidateAlternativeLibraryLocations(name: String): Array<String>
Copy link
Owner

Choose a reason for hiding this comment

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

I am not sure if I really understand this abstraction.

While it's nice to have the implementation separated by OS, this is fundamentally pretty specific to a certain kind of (fallback) library resolution, that shouldn't (IMO) live under a name as generic as org.javacs.kt.util.OSContext. I could imagine fitting ShellPathUtils into an interface like this, but that's a different use-case.

I think, in this case specifically I would probably even drop the interface/classes entirely and just inline the when-check over the OS with the corresponding paths into BackupClassPathResolver's findAlternativeLibraryLocation.

Alternatively, if the BackupClassPathResolver is getting too large, we might want to add a SnapClassPathResolver/ScoopClassPathResolver?

Copy link
Author

@tats-u tats-u Sep 3, 2023 •
edited

Choose a reason for hiding this comment

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

this is fundamentally pretty specific to a certain kind of (fallback) library resolution, that shouldn't (IMO) live under a name as generic as org.javacs.kt.util.OSContext. I could imagine fitting ShellPathUtils into an interface like this, but that's a different use-case.

I thought other methods that depend on the type of OS may be added in the future. I'll have to change their names.

I think, in this case specifically I would probably even drop the interface/classes entirely and just inline the when-check over the OS with the corresponding paths into BackupClassPathResolver's findAlternativeLibraryLocation.

I oppose it if it's called more than once through the entire of the program. We have to choose the appropriate instance according to user's OS. That that wasn't chosen mustn't be loaded.
The USERPROFILE value is used only in Windows, so it must be a state enclosed in the Windows class.

we might want to add a SnapClassPathResolver/ScoopClassPathResolver?

Do they mean singletons (objects) that implement ClassPathResolver?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@tats-u , I see that you have not been getting a reply here 🙁 I'm pretty sure that @fwcd meant creating singletons like you are asking.


companion object {
/**
* Gets the instance for the current OS
*/
val CURRENT_OS by lazy<OSContext> {
val osName = System.getProperty("os.name")!!.lowercase()
when {
osName.contains("windows") -> WindowsContext()
else -> UnixContext()
}
}
}
}
9 changes: 9 additions & 0 deletions shared/src/main/kotlin/org/javacs/kt/util/UnixContext.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package org.javacs.kt.util

/**
* Tasks for other than Windows
*/
class UnixContext : OSContext {
override fun candidateAlternativeLibraryLocations(name: String): Array<String> = // Snap (Linux)
arrayOf("/snap/kotlin/current/lib/${name}.jar")
}
20 changes: 20 additions & 0 deletions shared/src/main/kotlin/org/javacs/kt/util/WindowsContext.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package org.javacs.kt.util

/**
* Tasks only for Windows
*/
class WindowsContext : OSContext {
override fun candidateAlternativeLibraryLocations(name: String): Array<String> = // Scoop (https://scoop.sh)
CANDIDATE_PATHS.map {
"$it$name.jar"
}.toTypedArray()
companion object {
/**
* Absolute path to the user's profile folder (home directory)
*/
private val USERPROFILE = System.getenv("USERPROFILE")
private val CANDIDATE_PATHS = arrayOf(
"${USERPROFILE}\\scoop\\apps\\kotlin\\current\\lib\\",
)
}
}