feat(smartctl): update for smartmontools 7.5 #1471

chrfranke · 2025-10-23T13:54:59Z

This is a retry of #1425.

Uses a different approach to let menu-complete only append a space if all alternatives allow this.

akinomyoga · 2025-10-24T03:48:10Z

completions/smartctl

+    local i
+    for i in "${!COMPREPLY[@]}"; do
+        # shellcheck disable=SC2053
+        if [[ ${COMPREPLY[i]} == $1 ]]; then


I think we can fix the pattern (and remove the argument). The intent of the function is clearer this way.

Suggested change

if [[ ${COMPREPLY[i]} == $1 ]]; then

if [[ ${COMPREPLY[i]} == *[,=] ]]; then

The name of the function may also be adjusted.

akinomyoga · 2025-10-24T04:14:19Z

completions/smartctl

+        [1-9N],* | [1-9][0-9],* | 1[0-9][0-9],* | 2[0-4][0-9],* | 25[0-5],*)
+            fmt='raw{8,16,48,56,64},hex{48,56,64},raw24/raw{24,32}'
+            fmt+=',msec24hour32,{sec,min,halfmin}2hour,temp10x,tempminmax'
+            _comp_compgen -- -W '"${cur%%,*}",{'"$fmt"'}{,\,,}'


{,\,,} generates two empty suffixes. I guess

Suggested change

_comp_compgen -- -W '"${cur%%,*}",{'"$fmt"'}{,\,,}'

_comp_compgen -- -W '"${cur%%,*}",{'"$fmt"'}{,\,,:}'

Good catch.

akinomyoga · 2025-10-24T04:18:03Z

completions/smartctl

+    local fmt
+    case $cur in
+        [1-9N],* | [1-9][0-9],* | 1[0-9][0-9],* | 2[0-4][0-9],* | 25[0-5],*)
+            fmt='raw{8,16,48,56,64},hex{48,56,64},raw24/raw{24,32}'


man smartctl in my environment (Arch Linux with smartctl 7.5) also contains raw16(raw16), raw16(avg16), and raw24(raw8). What are they, and why are they excluded?

Again, good catch.

These options prints the raw value like 123 if the extra words are zero else like 123 (456 789) , see atacmds.cpp.

These were excluded because the (...) require shell quoting which didn't work. If added to _comp_compgen -- -W as raw16\$raw16\$, it correctly returns raw16$raw16$ but further completion does not work then. I also tried to pass raw16'('raw16')' with some \. Sorry if I missed something.

Please note that completeness is not very important here because these options were primarily added for the drivedb.h file and are rarely used from command line. Otherwise I wouldn't have chosen such syntax.

it correctly returns raw16$raw16$ but further completion does not work then.

I fetched your fork and checked the behavior. It seems the problem can be traced down to the fact that the compgen builtin performs the filtering under the assumption that the words specified to -W are the ones before the shell quoting.

$ compgen -W 'raw16\$raw16\$' -- 'raw16$' $ compgen -W 'raw16\\(raw16\$' -- 'raw16\$' raw16\(raw16$

One way is to generate it without quoting (i.e., -W 'raw16(raw16)') with compopt -o fullquote, but compopt -o fullquote is only available in Bash >= 5.3.

Bash >= 5.3 has been released end of July 2025. Even Ubuntu unstable ('questing') does not provide it yet. I suggest to exclude these three options for now.

Could you leave code comments about the three values?

Of course, something like this?

[1-9N],* | [1-9][0-9],* | 1[0-9][0-9],* | 2[0-4][0-9],* | 25[0-5],*) # 'raw16(raw16)', 'raw16(avg16)' and 'raw24(raw8)' are excluded # because these would require 'compopt -o fullquote' (Bash >= 5.3). # TODO: add these if Bash >= 5.3 is detected. fmt='raw{8,16,48,56,64},hex{48,56,64},raw24/raw{24,32}'

Yeah, but for lower versions of Bash, I'm actually thinking about incorporating __bat_escape_completions (e.g. into our _comp_compgen with a flag -q), so you don't have to describe how it would be solved in 5.3. But if you want to, you can.

akinomyoga · 2025-10-24T04:18:42Z

completions/smartctl

-            _comp_compgen -- -W 'help 9,minutes 9,seconds 9,halfminutes 9,temp
-                192,emergencyretractcyclect 193,loadunload 194,10xCelsius
-                194,unknown 198,offlinescanuncsectorct 200,writeerrorcount
-                201,detectedtacount 220,temp'


The man page says "The following old arguments to '-v' are also still valid:". Shouldn't we suggest these for users who are familiar with the old ones? Or maybe we can generate these as a fallback in the case that no completions are generated for the new arguments.

Sorry, no. It is planned for 8.0 to print a deprecation warning and later remove these in 8.2.

akinomyoga · 2025-10-24T04:27:43Z

completions/smartctl

+        aacraid,*,*,*)
+            _comp_compgen -- -W 'aacraid,{0..3},{0..7},{0..31}'
+            ;;
+        aacraid,*,*)
+            _comp_compgen -- -W 'aacraid,{0..3},{0..7},'
+            compopt -o nospace
+            ;;
+        sssraid,*,*)
+            _comp_compgen -- -W 'sssraid,{0..3},{0..31}'
+            ;;
+        aacraid,* | sssraid,*)
+            _comp_compgen -- -W '"${cur%%,*}",{0..3},'
+            compopt -o nospace
+            ;;


How about using _comp_compgen -P? Also, interleaving the completions for aacraid and sssraid seems a bit tricky to understand at a glance.

Suggested change

aacraid,*,*,*)

_comp_compgen -- -W 'aacraid,{0..3},{0..7},{0..31}'

;;

aacraid,*,*)

_comp_compgen -- -W 'aacraid,{0..3},{0..7},'

compopt -o nospace

;;

sssraid,*,*)

_comp_compgen -- -W 'sssraid,{0..3},{0..31}'

;;

aacraid,* | sssraid,*)

_comp_compgen -- -W '"${cur%%,*}",{0..3},'

compopt -o nospace

;;

aacraid,*,*,*,*) ;;

aacraid,*,*,*)

_comp_compgen -P "${cur%,*}," -- -W '{0..31}'

;;

aacraid,*,*)

_comp_compgen -P "${cur%,*}," -- -W '{0..7},'

compopt -o nospace

;;

aacraid,*)

_comp_compgen -P "${cur%,*}," -- -W '{0..3},'

compopt -o nospace

;;

sssraid,*,*,*) ::

sssraid,*,*)

_comp_compgen -P "${cur%,*}," -- -W '{0..31}'

;;

sssraid,*)

_comp_compgen -P "${cur%,*}," -- -W '{0..3},'

compopt -o nospace

;;

Sorry, no. This happily completes nonsense like:

$ /usr/sbin/smartctl -d aacraid,foo,bar,[TAB][TAB] aacraid,foo,bar,0 aacraid,foo,bar,16 aacraid,foo,bar,23 aacraid,foo,bar,30 ... aacraid,foo,bar,15 aacraid,foo,bar,22 aacraid,foo,bar,3 aacraid,foo,bar,9

Yes, I know that behavior, but I'm not sure whether we really need to care about the parts other than the currently completed part. Anyway, if you want to stop the completion when the previous part is wrong, it is also a valid decision. I was just concerned about the fact that aacraid,{0..3},{0..7},{0..31} internally generates 1000 candidates, which will finally be filtered out to less than or equal to 32 candidates always.

Anyway, the point you raised can be easily fixed by slightly modifying the patterns in the suggested code, e.g. from aacraid,*,*,* to aacraid,[0-3],[0-7],*.

OK.

BTW, even much larger expansions are still reasonably fast:

someone@debian-12:~$ time bash -c 'x=($(echo aacraid,{0..7},{0..15},{0..127})) ; echo "${x[@]}" | wc' 1 16384 254208 real 0m0.032s user 0m0.031s sys 0m0.004s

akinomyoga

Sorry, I meant "Request changes".

feat(smartctl): update for smartmontools 7.5

343d48a

chrfranke force-pushed the smartctl branch from ef3679a to 343d48a Compare October 23, 2025 14:24

akinomyoga approved these changes Oct 24, 2025

View reviewed changes

akinomyoga requested changes Oct 24, 2025

View reviewed changes

	if [[ ${COMPREPLY[i]} == $1 ]]; then
	if [[ ${COMPREPLY[i]} == *[,=] ]]; then

	_comp_compgen -- -W '"${cur%%,*}",{'"$fmt"'}{,\,,}'
	_comp_compgen -- -W '"${cur%%,*}",{'"$fmt"'}{,\,,:}'

feat(smartctl): update for smartmontools 7.5 #1471

Are you sure you want to change the base?

feat(smartctl): update for smartmontools 7.5 #1471

Conversation

chrfranke commented Oct 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrfranke Oct 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akinomyoga Oct 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akinomyoga Oct 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akinomyoga Oct 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akinomyoga left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chrfranke Oct 25, 2025 •

edited

Loading

akinomyoga Oct 25, 2025 •

edited

Loading

akinomyoga Oct 25, 2025 •

edited

Loading

akinomyoga Oct 25, 2025 •

edited

Loading