Skip to content

Commit 05b12f4

Browse files
iamgioiamgio
authored
Merge pull request #236 from iamgio/enhanced-browser-selection
feat(cli): enhance browser selection
2 parents 47a32ed + 251aa14 commit 05b12f4

File tree

7 files changed

+205
-78
lines changed

7 files changed

+205
-78
lines changed

‎quarkdown-cli/src/main/kotlin/com/quarkdown/cli/exec/CompileCommand.kt‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ import com.github.ajalt.clikt.parameters.options.option
66
import com.github.ajalt.clikt.parameters.types.file
77
import com.quarkdown.cli.CliOptions
88
import com.quarkdown.cli.exec.strategy.FileExecutionStrategy
9-
import com.quarkdown.cli.server.BrowserLaunchers.browserChoice
109
import com.quarkdown.cli.server.WebServerOptions
10+
import com.quarkdown.cli.server.browserLauncherOption
1111
import com.quarkdown.core.log.Log
1212
import com.quarkdown.core.pipeline.PipelineOptions
1313
import com.quarkdown.interaction.Env
@@ -48,11 +48,10 @@ class CompileCommand : ExecuteCommand("compile") {
4848
/**
4949
* Optional browser to open the served file in, if preview is enabled.
5050
*/
51-
private val browser: BrowserLauncher? by option(
52-
"-b",
53-
"--browser",
54-
help = "Browser to open the preview in",
55-
).browserChoice(default = DefaultBrowserLauncher(), shouldValidate = { preview })
51+
private val browser: BrowserLauncher? by browserLauncherOption(
52+
default = DefaultBrowserLauncher(),
53+
shouldValidate = { preview },
54+
)
5655

5756
/**
5857
* Session to communicate with the server in order to trigger reloads of the preview.

‎quarkdown-cli/src/main/kotlin/com/quarkdown/cli/server/BrowserLauncherOption.kt‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
package com.quarkdown.cli.server
2+
3+
import com.github.ajalt.clikt.core.CliktCommand
4+
import com.github.ajalt.clikt.parameters.options.OptionDelegate
5+
import com.github.ajalt.clikt.parameters.options.convert
6+
import com.github.ajalt.clikt.parameters.options.default
7+
import com.github.ajalt.clikt.parameters.options.option
8+
import com.quarkdown.core.log.Log
9+
import com.quarkdown.server.browser.BrowserLauncher
10+
import com.quarkdown.server.browser.DefaultBrowserLauncher
11+
import com.quarkdown.server.browser.EnvBrowserLauncher
12+
import com.quarkdown.server.browser.NoneBrowserLauncher
13+
import com.quarkdown.server.browser.PathBrowserLauncher
14+
import kotlin.io.path.Path
15+
16+
/**
17+
* Attempts to create a [BrowserLauncher] from fixed choices: `default` or `none`.
18+
* @param input the input string representing the browser choice
19+
* @return the corresponding [BrowserLauncher], if any
20+
*/
21+
private fun fromFixedChoices(input: String): BrowserLauncher? =
22+
when (input) {
23+
"default" -> DefaultBrowserLauncher()
24+
"none" -> NoneBrowserLauncher()
25+
else -> null
26+
}
27+
28+
/**
29+
* Attempts to create a [BrowserLauncher] from environment variables (e.g. `chrome` -> `BROWSER_CHROME`).
30+
* @param input the input string representing the browser choice (e.g. `chrome`, `firefox`)
31+
* @param envLookup function to look up environment variable values.
32+
* If different from `System::getenv`, it can be used for testing purposes
33+
* @return the corresponding [BrowserLauncher], if any
34+
*/
35+
private fun fromEnv(
36+
input: String,
37+
envLookup: (String) -> String?,
38+
): BrowserLauncher? =
39+
EnvBrowserLauncher(input, envLookup)
40+
.takeIf { it.isValid }
41+
?.also { Log.info("Using browser launcher $input (env ${it.envName})") }
42+
43+
/**
44+
* Attempts to create a [BrowserLauncher] from a given file system path.
45+
* @param input the input string representing the file system path to the browser executable
46+
* @return the corresponding [BrowserLauncher], if any
47+
*/
48+
private fun fromPath(input: String): BrowserLauncher? =
49+
PathBrowserLauncher(Path(input))
50+
.takeIf { it.isValid }
51+
?.also { Log.info("Using browser launcher from path: $input") }
52+
53+
/**
54+
* Option to select a browser launcher from the CLI,
55+
* with validation and support of selection by name, path, or fixed choices.
56+
* @param default the default browser launcher to use if no choice is made
57+
* @param shouldValidate whether the choice should be validated
58+
* @param envLookup function to look up environment variable values.
59+
* If different from `System::getenv`, it can be used for testing purposes
60+
*/
61+
fun CliktCommand.browserLauncherOption(
62+
default: BrowserLauncher = NoneBrowserLauncher(),
63+
shouldValidate: () -> Boolean = { true },
64+
envLookup: (String) -> String? = System::getenv,
65+
): OptionDelegate<out BrowserLauncher?> =
66+
option(
67+
"-b",
68+
"--browser",
69+
help = "Browser to open the served file in (name, path, 'default', 'none')",
70+
).convert { input ->
71+
val caseInsensitiveInput = input.lowercase()
72+
val launcher =
73+
fromFixedChoices(caseInsensitiveInput)
74+
?: fromEnv(caseInsensitiveInput, envLookup)
75+
?: fromPath(input)
76+
77+
require(!shouldValidate() || launcher != null) {
78+
"The specified browser ($input) cannot be launched " +
79+
"because it is either not installed, not loaded in the environment (BROWSER_<NAME>), " +
80+
"not executable, or unsupported."
81+
}
82+
83+
launcher ?: default
84+
}.default(default)

‎quarkdown-cli/src/main/kotlin/com/quarkdown/cli/server/BrowserLaunchers.kt‎

Lines changed: 0 additions & 50 deletions
This file was deleted.

‎quarkdown-cli/src/main/kotlin/com/quarkdown/cli/server/StartWebServerCommand.kt‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import com.github.ajalt.clikt.parameters.options.option
66
import com.github.ajalt.clikt.parameters.options.required
77
import com.github.ajalt.clikt.parameters.types.file
88
import com.github.ajalt.clikt.parameters.types.int
9-
import com.quarkdown.cli.server.BrowserLaunchers.browserChoice
109
import com.quarkdown.server.LocalFileWebServer
1110
import com.quarkdown.server.ServerEndpoints
1211
import com.quarkdown.server.browser.BrowserLauncher
@@ -41,11 +40,7 @@ class StartWebServerCommand : CliktCommand(name = "start") {
4140
/**
4241
* Optional browser to open the served file in.
4342
*/
44-
private val browser: BrowserLauncher? by option(
45-
"-b",
46-
"--browser",
47-
help = "Browser to open the served file in",
48-
).browserChoice()
43+
private val browser: BrowserLauncher? by browserLauncherOption()
4944

5045
override fun run() {
5146
val options = WebServerOptions(port, targetFile, browser, preferLivePreviewUrl = true)

‎quarkdown-cli/src/test/kotlin/com/quarkdown/cli/BrowserLauncherSelectionTest.kt‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
package com.quarkdown.cli
2+
3+
import com.github.ajalt.clikt.core.CliktCommand
4+
import com.github.ajalt.clikt.testing.test
5+
import com.quarkdown.cli.server.browserLauncherOption
6+
import com.quarkdown.server.browser.BrowserLauncher
7+
import com.quarkdown.server.browser.DefaultBrowserLauncher
8+
import com.quarkdown.server.browser.EnvBrowserLauncher
9+
import com.quarkdown.server.browser.NoneBrowserLauncher
10+
import kotlin.test.Test
11+
import kotlin.test.assertFails
12+
import kotlin.test.assertIs
13+
14+
/**
15+
* Mock command to test browser launcher selection.
16+
* Environment variables are simulated via [env].
17+
*/
18+
private class MockCommand(
19+
env: Map<String, String>,
20+
) : CliktCommand() {
21+
val browserLauncher by browserLauncherOption(envLookup = env::get)
22+
23+
override fun run() {}
24+
}
25+
26+
/**
27+
* Tests for browser launcher selection via [browserLauncherOption].
28+
*/
29+
class BrowserLauncherSelectionTest {
30+
private fun test(
31+
value: String? = null,
32+
env: Map<String, String> = emptyMap(),
33+
): BrowserLauncher? =
34+
MockCommand(env)
35+
.also {
36+
val argv =
37+
if (value != null) {
38+
arrayOf("--browser", value)
39+
} else {
40+
emptyArray()
41+
}
42+
it.test(argv)
43+
}.browserLauncher
44+
45+
@Test
46+
fun fallback() {
47+
assertIs<NoneBrowserLauncher>(test())
48+
}
49+
50+
@Test
51+
fun `default choice`() {
52+
assertIs<DefaultBrowserLauncher>(test("default"))
53+
}
54+
55+
@Test
56+
fun `none choice`() {
57+
assertIs<NoneBrowserLauncher>(test("none"))
58+
}
59+
60+
@Test
61+
fun `from env`() {
62+
val choice = "chrome"
63+
val envName = "BROWSER_${choice.uppercase()}"
64+
assertIs<EnvBrowserLauncher>(test(choice, env = mapOf(envName to "/path/to/chrome")))
65+
}
66+
67+
@Test
68+
fun `invalid from env`() {
69+
val choice = "nonexistentbrowser"
70+
assertFails { test(choice) }
71+
}
72+
73+
@Test
74+
fun `invalid from path`() {
75+
val path = "path/to/nonexistent/browser"
76+
assertFails { test(path) }
77+
}
78+
}

‎quarkdown-server/src/main/kotlin/com/quarkdown/server/browser/EnvBrowserLauncher.kt‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,33 +7,35 @@ package com.quarkdown.server.browser
77
private const val ENV_PREFIX = "BROWSER_"
88

99
/**
10-
* Launcher of browsers whose path is stored in environment variables (`BROWSER_<env>`).
10+
* Launcher of browsers whose path is stored in environment variables (`BROWSER_<env>`)
11+
* passing the URL as the first argument to the browser executable.
1112
*
12-
* @property env the environment variable suffix (e.g., "chrome" for BROWSER_CHROME).
13-
* @property envValue the resolved environment variable value if set.
13+
* @param env the environment variable suffix (e.g., `chrome` for `BROWSER_CHROME`).
14+
* @param envLookup function to look up environment variable values.
15+
* If different from `System::getenv`, it can be used for testing purposes
1416
*/
15-
abstract class EnvBrowserLauncher(
17+
class EnvBrowserLauncher(
1618
private val env: String,
19+
private val envLookup: (String) -> String?,
1720
) : BrowserLauncher {
21+
/**
22+
* The name of the environment variable for the browser.
23+
*/
24+
val envName: String
25+
get() = ENV_PREFIX + env.uppercase()
26+
1827
/**
1928
* The value of the environment variable for the browser, if set.
2029
*/
21-
protected val envValue: String?
22-
get() = System.getenv(ENV_PREFIX + env.uppercase())
23-
}
30+
private val envValue: String? by lazy {
31+
this.envLookup(envName)
32+
}
2433

25-
/**
26-
* Simple implementation of [EnvBrowserLauncher] that launches a browser if the environment variable is set,
27-
* passing the URL as the first argument to the browser executable.
28-
*/
29-
open class SimpleEnvBrowserLauncher(
30-
env: String,
31-
) : EnvBrowserLauncher(env) {
3234
override val isValid: Boolean
33-
get() = super.envValue != null && super.envValue!!.isNotBlank()
35+
get() = this.envValue.isNullOrBlank().not()
3436

3537
override fun launch(url: String) {
36-
val browserPath = super.envValue!!
38+
val browserPath = this.envValue!!
3739
val command = arrayOf(browserPath, url)
3840
Runtime.getRuntime().exec(command)
3941
}

‎quarkdown-server/src/main/kotlin/com/quarkdown/server/browser/PathBrowserLauncher.kt‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package com.quarkdown.server.browser
2+
3+
import java.nio.file.Path
4+
5+
/**
6+
* Browser launcher that uses a specific file system path to launch the browser.
7+
* @param path the file system path to the browser executable
8+
*/
9+
class PathBrowserLauncher(
10+
private val path: Path,
11+
) : BrowserLauncher {
12+
override val isValid: Boolean
13+
get() = path.toFile().run { exists() && canExecute() }
14+
15+
override fun launch(url: String) {
16+
val processBuilder = ProcessBuilder(path.toAbsolutePath().toString(), url)
17+
processBuilder.start()
18+
}
19+
}

0 commit comments

Comments
 (0)