feat/regex-search-support #153
Conversation
Added minimal usage of `j` and `k` for navigation across the logs
|
fixed PR title |
|
Thank you for the effort, I tried something, but am not sure if this is correct (am just mechanical engineer, user, not skilled in coding). Seems correct version is installed as there's Tried some minimal working example (mwe.log): Result: |
|
would look into it, @petric3 could you please provide some kind of sanitized (without any sensitive data) log file ( |
|
I did attach the minimal working file, here it goes in raw version: Is this good enough? |
|
@petric3 Hello, I used your logs provided above and it seems to be working 
UPD: Nevermind, could reproduce it, it seems to be case-related issue |
|
Updated it, now regexp seems to filter correctly 
|
|
@winterka hey, I saw it and then reinstalled it and now it works also for me, great 👍 I've install a non-main version for the first time :) I guess did something wrong. Great, hopefully it can be merged into main version soon. I'm gonna close the issue once merged. Thank you once more 🙏 |
hedhyw
left a comment
hedhyw
left a comment
There was a problem hiding this comment.
praise: thank you for your contribution!
thought: regexes are powerful, but they seem too powerful and not very convenient for a TUI (easy to make a mistake). What about globs? https://github.com/gobwas/glob, which support simple abc*xyz or more advanced AND patterns {abc,xyz,123}. It's just a thing to discuss, I'm not saying it's the right way :)
note: the PR title should follow conventional commits:
https://www.conventionalcommits.org/en/v1.0.0/
it should be: feat: regex search support
| }, nil | ||
| } | ||
|
|
||
| // FilterRegEx filters entries by regex ignoring case |
There was a problem hiding this comment.
| // FilterRegEx filters entries by regex ignoring case | |
| // FilterRegEx filters entries by regex ignoring case. |
It will fix a linter issue, you can try make lint
| return s.BaseStyle.Render(s.table.View()) + "\n" + footer | ||
| } | ||
|
|
||
| func (s StateRegexFilteredModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { |
There was a problem hiding this comment.
nitpick: can you please add comments to all exported methods?
There was a problem hiding this comment.
thought: it's more flexible to have a dedicated state for regexp filtering - StateRegexFilteredModel (as you implemented). However it introduces a lot of boilerplate and still overlaps with StateFilteredModel. What about extending the latter? newStateFiltered -> newStateFilteredByText(previousState, filterText) and newStateFilteredByRegexp(previousState, filterRegexp).
| return s, tea.Quit | ||
| case key.Matches(msg, s.keys.Filter): | ||
| return s.handleFilterKeyClickedMsg() | ||
| case key.Matches(msg, s.keys.FilterRegex): |
| return entries, nil | ||
| } | ||
|
|
||
| re, err := regexp.Compile(strings.ToLower(regex)) |
There was a problem hiding this comment.
note: there’s a (?i) flag for case-insensitive matching, you just need to add it to the start of the regex if it isn't already there.
(?i)[a-z0-9]+ will match both AAAAA and aaaaaa
| return LazyLogEntries{}, err | ||
| } | ||
|
|
||
| if re.Match([]byte(bytes.ToLower(line))) { |
There was a problem hiding this comment.
note: we don't need to call ToLower if you include the (?i) flag
| return entries, nil | ||
| } | ||
|
|
||
| re, err := regexp.Compile(strings.ToLower(regex)) |
There was a problem hiding this comment.
question: how will errors be reported? It would be annoying to loose everything after a small mistake in a regexp
It's hard for me to comment on this one, really am just a simple user from this side. From my perspective, I use pretty big log files, so I'd simply go for what's faster solution. |
3 participants
Resolving this one: #152
Tried to follow your code-style, architecture and approaches. There might and I guess there are some naming issues, feel free to point them out and I'd fix them if needed.