Skip to content

Conversation

chrfranke
Copy link

A followup to #1425 with two commits addressing independent topics.

Should now detect these properly:

  -n MODE[,STATUS[,STATUS2]], --nocheck=MODE[,STATUS[,STATUS2]] (ATA, SCSI)
  -s NAME[,VALUE], --set=NAME[,VALUE]
  -B [+]FILE, --drivedb=[+]FILE                                       (ATA)

@chrfranke chrfranke force-pushed the smartctl-_comp_compgen_help branch from c422102 to b53b143 Compare September 16, 2025 12:45
@chrfranke
Copy link
Author

I don't think it's worth such a complication. What I had in mind is to allow + as one of the possible characters in the option-argument description...

Force-pushed new version of 2nd commit.

Copy link
Collaborator

@akinomyoga akinomyoga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@chrfranke
Copy link
Author

Sorry for CI failures. Will address these in the next days.

@chrfranke chrfranke force-pushed the smartctl-_comp_compgen_help branch from d083fae to c3c8648 Compare September 18, 2025 15:45
@chrfranke
Copy link
Author

Force-pushed new version, now combined in one commit. This reformats (as suggested by pre-commit) and fixes the testcases.

--- a/test/t/unit/test_unit_parse_help.py
+++ b/test/t/unit/test_unit_parse_help.py
@@ -205,15 +205,19 @@ class TestUnitParseHelp:

     def test_33(self, bash):
         """Pattern found in the output of 'smartctl --help'."""
-        assert_bash_exec(bash, "fn() { echo '-f A1[,A2[,A3]] --foo=A1[,A2[,A3]]'; }")
+        assert_bash_exec(
+            bash, "fn() { echo '-f A1[,A2[,A3]] --foo=A1[,A2[,A3]]'; }"
+        )
         output = assert_bash_exec(bash, "_parse_help fn", want_output=True)
-        assert output.split() == "--foo".split()
+        assert output.split() == "--foo=".split()

     def test_34(self, bash):
         """Pattern found in the output of 'smartctl --help'."""
-        assert_bash_exec(bash, "fn() { echo '-f [+]ARG --foo=[+]ARG'; }")
+        assert_bash_exec(
+            bash, "fn() { echo '-f [+]ARG --foo=[+]ARG'; }"
+        )
         output = assert_bash_exec(bash, "_parse_help fn", want_output=True)
-        assert output.split() == "--foo".split()
+        assert output.split() == "--foo=".split()

     def test_custom_helpopt1(self, bash):
         assert_bash_exec(bash, "fn() { [[ $1 == -h ]] && echo '-option'; }")

@chrfranke chrfranke force-pushed the smartctl-_comp_compgen_help branch from c3c8648 to de6b0cb Compare September 19, 2025 10:30
@chrfranke
Copy link
Author

Obviously my assumption about max allowed title length (4 indentation + 74 title = 78) was wrong. Force-pushed a new version using the original title of this PR. The code is unchanged.

Allow plus sign and nested braces in short option argument.
@chrfranke chrfranke force-pushed the smartctl-_comp_compgen_help branch from de6b0cb to 19300b5 Compare September 19, 2025 11:39
@chrfranke
Copy link
Author

Undoing reformatting of test_34. Last try for this week to make pre-commit happy :-)

@akinomyoga
Copy link
Collaborator

The CI error in fedoradev seems to have started to happen after the latest update-docker-images and is unrelated to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants