Conversation
backwardspy
left a comment
There was a problem hiding this comment.
nice work! just a couple of suggestions to consider.
| def parse_config(config: str, verbose: bool) -> list[TmuxCommand]: | ||
| commands: list[TmuxCommand] = [] | ||
| # TODO: Handle multiline commands that contain '\' | ||
| for line_number, line in enumerate(config.splitlines()): |
There was a problem hiding this comment.
| for line_number, line in enumerate(config.splitlines()): | |
| for line_number, line in enumerate(config.splitlines(), start=1): |
to start counting lines at 1 rather than 0, unless i've missed a +1 somewhere else!
| # TODO: Handle multiline commands that contain '\' | ||
| for line_number, line in enumerate(config.splitlines()): | ||
| cmd_re = r"\s*(\w+)\s+(-\w+)?\s*(@?[\w\-_]+)?\s*[\"']?(.*)" | ||
| m = re.match(cmd_re, line) |
There was a problem hiding this comment.
you can re.compile this pattern once at the beginning and then match with that, rather than recompiling on each iteration of the loop
There was a problem hiding this comment.
Why is that a benefit?
There was a problem hiding this comment.
it might not be. in theory, compiling it once and then using it many times should be kinder on system resources. however, i believe python maintains its own cache for compiled regex patterns anyway, so you may see no difference at all.
i'm also not sure how long the average tmux config is; if this loop only runs a few hundred times it would be a negligible difference.
thought it was worth pointing out for consideration regardless.
edit: i do see some improvement locally:
$ python -m timeit -s 'import re' 're.match("[a-z]", "hello world!")'
1000000 loops, best of 5: 227 nsec per loop
$ python -m timeit -s 'import re; pat = re.compile("[a-z]")' 'pat.match("hello world!")'
5000000 loops, best of 5: 83.3 nsec per loopThere was a problem hiding this comment.
It is cached, and I'm not really interested in micro optimisations without a measurable performance benefit that users would notice
There was a problem hiding this comment.
as i said, just a suggestion :)
No description provided.