Skip to content

mrview: fix handling of -focus and -target options#2250

Merged
jdtournier merged 7 commits intomasterfrom
mrview_fix_option_handling_focus_target
Jan 20, 2021
Merged

mrview: fix handling of -focus and -target options#2250
jdtournier merged 7 commits intomasterfrom
mrview_fix_option_handling_focus_target

Conversation

@jdtournier
Copy link
Member

@jdtournier jdtournier commented Jan 6, 2021

  • The -focus option was not labelled as type_sequence_float(), leading to
    a warning if the first value is negative, e.g.:

    mrview: [WARNING] Value "-1,2,3" is being used as the expected argument
    for option "-focus"; is this what you intended?
    
  • It should be allowed to use the -target option multiple times.

- The -focus option was not labelled as type_sequence_float(), leading to
  a warning if the first value is negative, e.g.:

      mrview: [WARNING] Value "-1,2,3" is being used as the expected argument
      for option "-focus"; is this what you intended?

- It should be allowed to use the -target option multiple times.
@jdtournier jdtournier added this to the 3.0.3 hotfix milestone Jan 6, 2021
@jdtournier jdtournier requested a review from a team January 6, 2021 12:45
@jdtournier jdtournier self-assigned this Jan 6, 2021
Copy link
Member

@Lestropie Lestropie left a comment

Choose a reason for hiding this comment

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

Given that -focus can be either a floating-point sequence or a boolean, it would be better flagged as .type_various(). That will still remove the erroneous warning if the first character of the provided argument is a dash.

@jdtournier
Copy link
Member Author

it would be better flagged as .type_various().

I guess you learn something new every day... 👍

@jdtournier jdtournier merged commit 8cd105a into master Jan 20, 2021
@jdtournier jdtournier deleted the mrview_fix_option_handling_focus_target branch January 20, 2021 13:34
@Lestropie Lestropie modified the milestones: 3.0.4 hotfix, 3.0.3 hotfix May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants