Updated regex for extracting '-l' and '-j' args for make #573
Conversation
Summary of improvements: - No longer consuming args that happen to begin `jX` or `lX` (where `X` is a sequence if integers. - Fixed bug where an empty 'j' arg would throw a `ValueError` when trying to convert an empty string (`''`) to an int - Both `-l/--load-average` and `-j/--jobs` can now be empty (which is supported by GNU Make - The `-l/--load-average` arg will now accept a float value (e.g. `catkin build -l4.0`) which is supported by GNU Make
|
This also addresses #516. |
|
This seems like a good improvement, but I'll be honest those regexes kind of terrify me. Would you be willing to contribute along with this a small test that demonstrates expected inputs/outputs? |
|
They terrify me too! They aren't that different from what was already there, so I figured just modifying the regex (instead of a complete re-write) would be less challenging to get accepted. Happy to contribute a few tests. I'll update the PR in a few days. |
Test cases also helped to make regexes slightly more robust.
|
It took over a month instead of a few days, but I finally pushed a few tests. Wouldn't you know it, the unit tests revealed a few additional minor flaws in the regular expressions that were in use. The current version seems to pass all corner cases that I could think of. |
|
Thank you so much for this contribution and your patience in awaiting a merge. |
Addresses #555
Summary of improvements:
jXorlX(whereXis asequence if integers.
ValueErrorwhen trying toconvert an empty string (
'') to an int-l/--load-averageand-j/--jobscan now be empty (which issupported by GNU Make
-l/--load-averagearg will now accept a float value (e.g.catkin build -l4.0) which is supported by GNU Make