amanusk / amanusk/s-tui

--refresh-rate is ignored when a saved configuration exists

オープン
#306 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
Python
スター
5.1k
フォーク
181
平均マージ
30分
マージ済み PR(30日)
2

説明

### Step 1: Describe your environment

- OS: ALT Linux
- s-tui versions tested: 1.1.4 and 1.5.0
- Installation method: distribution package

### Step 2: Describe the problem

The `--refresh-rate` / `-r` command-line option is ignored when the user has previously saved a refresh rate in the s-tui configuration.

The option was introduced in #196 to allow setting the refresh rate at launch, including for automated workflows. However, its value is currently overwritten by `~/.config/s-tui/s-tui.conf`.

### Step 3: Reproduce the problem

1. Start `s-tui`.
2. Open `Visual options`.
3. Set `Refresh` to `5`.
4. Select `Save settings` and quit.
5. Run:

```console
s-tui --refresh-rate 1
```

### Observed result

The refresh rate remains `5` seconds, as stored in the configuration file.

### Expected result

The explicitly supplied command-line value should take precedence, so the refresh rate should be `1` second.

The expected precedence is:

```text
built-in default < saved configuration < command-line argument
```

### Root cause

`GraphController.__init__()` initially assigns the command-line value:

```python
self.refresh_rate = args.refresh_rate
```

It then calls `_load_config()`, which unconditionally overwrites it when the configuration contains `GraphControl.refresh`:

```python
self.refresh_rate = str(self.conf.getfloat("GraphControl", "refresh"))
```

The parser also uses `"2.0"` as the argument default, so the application cannot distinguish between an omitted option and an explicit `--refresh-rate 2.0`.

The temperature threshold option already implements the expected CLI-over-config precedence.

### Suggested fix

Use `None` as the argparse default to represent an omitted option, and apply values in this order:

1. Initialize the built-in default (`2.0`).
2. Load the saved configuration.
3. If `args.refresh_rate is not None`, apply it last.

Simply moving the existing assignment after `_load_config()` would not be sufficient because the current argparse default would then overwrite the saved configuration even when the option was omitted.

Suggested test cases:

- no configuration and no `-r`: use `2.0`;
- saved configuration and no `-r`: use the saved value;
- saved configuration plus `-r 1`: use `1`;
- saved configuration plus explicit `-r 2.0`: use `2.0`.

Downstream report: https://bugzilla.altlinux.org/48524

コントリビューションガイド

コントリビューションガイドを開く

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。