feat(smartctl): update for smartmontools 7.5 #1425
Conversation
|
Force-pushed two fixes: added missing |
|
|
||
| if [[ $cur == -* ]]; then | ||
| _comp_compgen_help | ||
| # _comp_compgen_help # not used because result is incomplete |
There was a problem hiding this comment.
We try to avoid hardcoding as much as possible because a hardcoded list cannot reflect version differences. Couldn't the output of smartctl --help be fixed so that a complete list of options can be extracted?
There was a problem hiding this comment.
Yes - for the long term as this is already planned for next release. But next release 8.0 will take plenty of time due to planned refactoring and enhancements after we finally moved the project from SF svn to GH three month ago.
So please accept the hardcoding for now to support existing releases.
Please note that the current coverage of options is really outdated. For example the -v, --vendorattribute completions only cover options which are deprecated since I added the more flexible -v ID,RAW_FORMAT,ATTR_NAME syntax in smartmontools 5.39. This version was released in December 2009.
There was a problem hiding this comment.
I'm talking about the above line of removing _comp_compgen_help. I'm not talking about the list of possible values for option arguments of each option (though it would be better if the smartctl binary could provide a list of possible values in 8.0). I've checked the output of _comp_compgen_help, and the only missing options are --drivedb, --nocheck, and --set, where the following lines in the output --help seem to fail to be parsed.
-n MODE[,STATUS[,STATUS2]], --nocheck=MODE[,STATUS[,STATUS2]] (ATA, SCSI)
-s NAME[,VALUE], --set=NAME[,VALUE]
-B [+]FILE, --drivedb=[+]FILE (ATA)
This may be fixed at the side of _comp_compgen_help by allowing a wider format of the option argument name at line 1697 of bash_completion.
| --vendorattribute= --version --xall -A -B -C -F -H -P -S -T -V -X | ||
| -a -b -c -d -f -g -h -i -j -l -n -o -q -r -s -t -v -x' |
There was a problem hiding this comment.
Please check CONTRIBUTING.md; remove short options that have corresponding long options.
There was a problem hiding this comment.
Of course. Sorry, I missed that note.
| _comp_compgen -- -W '{1..9} N, help' | ||
| ;; | ||
| esac | ||
| [[ ${COMPREPLY-} == *, ]] && compopt -o nospace |
There was a problem hiding this comment.
- Doesn't this cause insertion of a space for
{1..9}($cur in *) and${cur}{0..9}($cur in [1-9]) withmenu-complete, etc?
There was a problem hiding this comment.
Indeed. Let me re-think this part...
There was a problem hiding this comment.
... see new _comp_cmd_smartctl__vendorattribute() below.
| _comp_compgen -- -W '${cur}, {100..255},' | ||
| ;; | ||
| [1-9]) | ||
| _comp_compgen -- -W '${cur}, ${cur}{0..9}' |
There was a problem hiding this comment.
| _comp_compgen -- -W '${cur}, ${cur}{0..9}' | |
| _comp_compgen -- -W '"$cur", "$cur"{0..9}' |
There was a problem hiding this comment.
For all uses of $cur within the _comp_compgen -- -W ... parameter or only this one?
| _comp_compgen_filedir h && [[ $prefix ]] && | ||
| _comp_compgen -Rv COMPREPLY -- -P "$prefix" -W '"${COMPREPLY[@]}"' | ||
| } | ||
| _comp_cmd_smartctl__vendorattribute() |
There was a problem hiding this comment.
This function seems to try to assist in inputting a number {0..255} by suggesting one digit at a time, but it can be confusing. For example, when $cur is empty, it only lists {1..9} (which is actually merely the first digit of possible numbers {0..255}), but it looks like the accepted number range is {1..9}.
There was a problem hiding this comment.
Alternative: Drop the completion of digits and continue completion after -v {1..255}, or -v N, is present:
_comp_cmd_smartctl__vendorattribute()
{
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}'
fmt+=',msec24hour32,{sec,min,halfmin}2hour,temp10x,tempminmax'
_comp_compgen -- -W '${cur%%,*},{'"$fmt"'}{,\,,}'
;;
*)
_comp_compgen -- -W 'N, help'
;;
esac
[[ ${COMPREPLY-} == *, ]] && compopt -o nospace
}This would also fix the insertion of a space for ... with menu-complete you mentioned above.
There was a problem hiding this comment.
Or
;;
+ [1-9])
+ _comp_compgen -- -W '{1..255},'
+ ;;
*)
_comp_compgen -- -W 'N, help'
;;though 1TAB will surpass the default bind 'set completion-query-items 100'.
| ;; | ||
| hpt*) | ||
| aacraid,*) | ||
| _comp_compgen -- -W 'aacraid,{0..1},{0..1},{0..7}' |
There was a problem hiding this comment.
This isn't specific to this PR but also applies to existing completions for hpt,*, but I'm wondering if it is possible to let the user input each section (separated by commas) incrementally.
There was a problem hiding this comment.
Could you point to an example in another completion file?
There was a problem hiding this comment.
By "incrementally", I mean that, as you did in _comp_cmd_smartctl__vendorattribute, it first generate the completions "${cur%,*}",{0..1}, when cur starts with aacraid, but without any additional comma. When there is already aacraid,[01], in the command line, it generates the next completions "${cur%,*}",{0..1},. When there is aacraid,[01],[01], in the command line, it generates the next completions "${cur%,*}",{0..7},.
|
Thanks for reviewing and various hints. #1430 should be addressed first. I will provide a reworked version of |
Backward compatible with even 5.* (<= 2012) versions of smartctl, except for the newly added options.