refactor: give each way of opening a URL a name - #63
Merged
Conversation
The switch in command() held the argv for every platform inline, so the list of programs octoscope may run was spread across the branches of one function. Each is now a type with a name, and the whole list reads off the openers slice. The dispatch did not go away, it moved: what was a case is now an early return inside each opener. What ordering used to get from the switch reading top to bottom it now gets from the slice, so the two places the order is load-bearing are pinned by tests -- WSL before plain Linux, and a browser the user named before every platform default. The latter was untested on Windows and macOS, where alone it can be observed. The environment variables are read through named constants and gathered into one environment value at the call, so what this package takes from the machine is visible at the top of the file instead of at three scattered os.Getenv calls. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Code Metrics Report
Code coverage of files in pull request scope (55.8%, patch 80.0%)
Reported by octocov |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
なぜ
internal/browserのcommand()が、プラットフォームごとの argv を switch の各 case に直書きしていた。octoscope が起動しうるプログラムの一覧が 1 つの関数の分岐に散っていて、何が試されるのかが読み取りにくい。何をしたか
openerinterface を切り、ブラウザの起動方法ごとに型を用意した。nilが「自分の出番ではない」を意味する。環境変数は名前付き定数にまとめ、
goos/wsl/browserを持つenvironment値として呼び出し時に 1 度だけ組み立てる。取得タイミングは変えていない(initには移していない)。このパッケージがマシンから何を読むかがファイル冒頭で一覧できる。トレードオフ
分岐は消えていない。置き場所が変わっただけ。
case goos == "windows":が各型のif env.goos != osWindows { return nil }になり、実装は 25 行から約 95 行に増えた。得たのは「コマンドに名前が付く」ことと「一覧が 1 箇所にまとまる」こと。順序について
switch の上から下への読み順が持っていた優先順位が、スライスの並びという暗黙の前提になった。壊れうる箇所なので、テストで両方向を固定した。
既存テストは
goos: linuxでしか「ユーザー指定のブラウザが勝つ」を見ておらず、userNamed{}を後ろに下げても通ってしまう穴があった(Linux では他の opener がnilを返すため順序が結果に出ない)。Windows と macOS のケースを追加した。wslBridgeとxdgOpenを入れ替えuserNamedを後ろへGH_BROWSER/BROWSERを入れ替え確認
make check終了コード 0(tidy / lint 0 issues / fmt / 883 tests)🤖 Generated with Claude Code