Skip to content
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.

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

status regex includes non-whitespace #1941

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tfchristie-wi
Copy link

No description provided.

@mjbear
Copy link
Contributor

mjbear commented Dec 20, 2024

resolves #1938

@mjbear
Copy link
Contributor

mjbear commented Dec 20, 2024

It doesn't appear that .* is too loose (greedy) based on how your test data behaves when parsed. 🙂

Copy link
Contributor

@jmcgill298 jmcgill298 left a comment

Choose a reason for hiding this comment

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

@tfchristie-wi
Copy link
Author

The test that fails seems to be one where keywords are in the description/name.
I went back and looked at the regex for NAME and it is: Value NAME (.+?)
I am not even sure how to parse that. It seems like a field length: (.{18})\s would be more sane.
changing :?.*\s to :?.{0,9}\s seems to work as well.
NB: the test file: cisco_ios_show_interfaces_status_with_keywords_in_name.raw
has 3 lines for the same interface:

Fa0/4 uplink notconnect 123 auto auto 10/100BaseTX
Fa0/4 downlink notconnect 456 auto auto 10/100BaseTX
Fa0/4 suspended customer disabled 789 auto auto 10/100BaseTX

@mjbear
Copy link
Contributor

mjbear commented Dec 21, 2024

The test that fails seems to be one where keywords are in the description/name. I went back and looked at the regex for NAME and it is: Value NAME (.+?) I am not even sure how to parse that.

There are different lines for the various matches.
I'll take to this and submit some changes your way @tfchristie-wi

It seems like a field length: (.{18})\s would be more sane. changing :?.*\s to :?.{0,9}\s seems to work as well. NB: the test file: cisco_ios_show_interfaces_status_with_keywords_in_name.raw has 3 lines for the same interface:

I'll see what I can work up - hopefully something simple. 😁

Fa0/4 uplink notconnect 123 auto auto 10/100BaseTX Fa0/4 downlink notconnect 456 auto auto 10/100BaseTX Fa0/4 suspended customer disabled 789 auto auto 10/100BaseTX

It is mostly cosmetic that there are three of the same ... though it could be helpful to have them be unique port names.

@mjbear
Copy link
Contributor

mjbear commented Dec 21, 2024

@tfchristie-wi
I submitted tfchristie-wi/pull/1 for you to review.

I include the below workflow description in case you're new to git and GitHub.
Once you've reviewed my PR against your fix-issue-1938 branch, you can approve it which will merge my changes into your branch thus making it part of your upstream NTC templates 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.

cisco_ios_show_interfaces_status.textfsm throwing error when status has additional text
3 participants