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

feat: add ignore_pattern option to filter language lines #217

Merged
merged 3 commits into from
Mar 3, 2025

Conversation

kanghengliu
Copy link
Contributor

@kanghengliu kanghengliu commented Mar 3, 2025

Refs

#216

This PR adds an option to ignore matched patterns(for example: ipython cell magics) to send to LSP.

Todo:

  • match a pattern to ignore for each languages
  • match multiple patterns to ignore (prefer pattern matching once instead)

@jmbuhr
Copy link
Owner

jmbuhr commented Mar 3, 2025

Looks good! Small nitpick would be that the docstring is slightly wrong, lua match patterns are not exactly the same es regexes. But I can fix that later.

Also a heads up: We now have so many arguments to otter activate that I will refactor it into using a table of keyword arguments, so how you call it will change. But you should get a deprecation warning then.

@kanghengliu
Copy link
Contributor Author

docstring was an oversight whoops!

I'm also trying to expand the match_pattern for a language into a table of strings to match multiple patterns, since lua pattern matching is limited in operators. (for instance OR | does not exist)

@jmbuhr
Copy link
Owner

jmbuhr commented Mar 3, 2025

Maybe ^[%%!] is already sufficient? This would cover all single-character magic things.
image

But I'm open to a list of patterns as well for more flexibility.

@kanghengliu
Copy link
Contributor Author

Yes indeed it is. I'm actually leaning to scraping the idea. Matching on multiple string might? slow down ls collection. Right now ^(%s*[%%!].*) works pretty well for possible magic/shell commands I can think of.
Screenshot 2025-03-03 at 8 16 47 AM

Ignored pattern is matched as language line collection
Prefer to not remove lines if there's a match in 'ignore_patten'
Ref: jmbuhr#217
@kanghengliu kanghengliu requested a review from jmbuhr March 3, 2025 16:16
Copy link
Owner

@jmbuhr jmbuhr left a comment

Choose a reason for hiding this comment

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

one last thing. This will fail if there is no pattern for a language, so pattern will be nil and string.match will fail. So we should only attempt to match if pattern is not nil.

'ignore_pattern` for a language can be nil, causing
string.match to fail.
This commit adds checks before attemping to match.

Ref: jmbuhr#217
@kanghengliu kanghengliu requested a review from jmbuhr March 3, 2025 17:14
@jmbuhr jmbuhr merged commit 4ac831e into jmbuhr:main Mar 3, 2025
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.

2 participants