Skip to content

T7758: allow repeated option --type in list_interfaces #42

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
jestabro merged 1 commit into from
Aug 28, 2025
Merged

T7758: allow repeated option --type in list_interfaces #42

jestabro merged 1 commit into from
Aug 28, 2025

Conversation

@jestabro
Copy link
Contributor

@jestabro jestabro commented Aug 27, 2025 •
edited
Loading

Change summary

Feature request: simple extension to allow repeated option --type:

vyos@vyos:/usr/libexec/vyos/completion$ ./list_interfaces --type ethernet --type dummy
eth0 eth1 eth2 eth3 dum0 dum1

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

Related PR(s)

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

@jestabro jestabro self-assigned this Aug 27, 2025
@jestabro jestabro requested a review from dmbaturin August 27, 2025 14:01
dmbaturin
dmbaturin approved these changes Aug 28, 2025
View reviewed changes
Copy link
Member

@dmbaturin dmbaturin left a comment

Choose a reason for hiding this comment

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

This is certainly a missed useful case. I left a couple of comments, neither is an obstacle for merging.

src/completion/list_interfaces/list_interfaces.ml
in out @ add_out

let get_interfaces =
let types = List.rev !intf_types in
Copy link
Member

@dmbaturin dmbaturin Aug 28, 2025

Choose a reason for hiding this comment

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

Just a thought: in this implementation, interfaces will appear in the order of the completion helper options. In many cases, our completion outputs are sorted lexicographically, so I wonder if it should also be done here so that --type ethernet --type dummy produces dum0 dum1 eth0 eth1 rather than eth0 eth1 dum0 dum1.

I don't see this as an obstacle for merging this PR, I'm just wondering what behavior we ultimately want here.

Copy link
Contributor Author

@jestabro jestabro Aug 28, 2025

Choose a reason for hiding this comment

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

Yes, this was considered, and the choice made to leave this in the hands of the user of the completion helper.

@dmbaturin dmbaturin changed the title T7758: allow repeated option --type T7758: allow repeated option --type in list_interfaces Aug 28, 2025
@jestabro jestabro force-pushed the list-interfaces-multiple-types branch from 8df04e8 to ac2ee23 Compare August 28, 2025 12:46
@jestabro jestabro merged commit 51ac77c into vyos:current Aug 28, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dmbaturin dmbaturin dmbaturin approved these changes

Assignees

@jestabro jestabro

Labels

mirror-completed

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jestabro @dmbaturin @vyosbot