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(highlight): allow extended highlighting patterns (#185) #255

Merged
merged 1 commit into from
Jul 6, 2024

Conversation

LunarLambda
Copy link
Contributor

@LunarLambda LunarLambda commented Mar 11, 2024

Highlight the first capture group pattern, if there is a second (typically nested) capture group, use it for keyword matching.
Allows highlighting TODO(foo) using .*<((KEYWORDS)%(\(.{-1,}\))?):.

Closes #185, closes #10

This is similar to #180, however that PR's matching behaviour is not backwards compatible with the default highlight pattern used. This PR retains the behaviour that only the first capturing group is highlighted.

@LunarLambda LunarLambda mentioned this pull request Mar 11, 2024
1 task
@LunarLambda LunarLambda changed the title feat(highlight): allow complex keyword patterns (#185) feat(highlight): allow extended highlighting patterns (#185) Mar 11, 2024
@YuanYuYuan
Copy link

YuanYuYuan commented Apr 11, 2024

Hi there! This PR is very helpful to me and doesn't seem to cause any obvious issues. Are there any concerns before merging?

@LunarLambda
Copy link
Contributor Author

No concerns were raised here or in the original issue(s), the plugin maintainer just hasn't responded yet.

My PR branch tracks master, you could switch over to it for the time being.

@YuanYuYuan
Copy link

My PR branch tracks master, you could switch over to it for the time being.

Hi @LunarLambda , yes, that's also my workaround right now. Thanks for your PR. And it's nice to have this merged. 😄 .

BTW, we may need to extend the search pattern as well. So I add the following pattern in my config

require("todo-comments").setup({
  highlight = {
    -- vimgrep regex, supporting the pattern TODO(name):
    pattern = [[.*<((KEYWORDS)%(\(.{-1,}\))?):]],
  },
  search = {
    -- ripgrep regex, supporting the pattern TODO(name):
    pattern = [[\b(KEYWORDS)(\(\w*\))*:]],
  }
})

@LunarLambda
Copy link
Contributor Author

Rebased against latest master.

@dpetka2001
Copy link
Contributor

Would be nice to see this functionality in main plugin. Been using OP's branch without any apparent problems so far.

@LunarLambda
Copy link
Contributor Author

It makes me happy that the PR is so well received. I will continue to rebase it against main since it's such a small change.

@sebszyller
Copy link

This has been waiting since March, and even Feb if you count #180. Any chance of merging so we can stop using OP's branch

@LunarLambda
Copy link
Contributor Author

I've previously tried to notify folke once or twice in the issue where I originally proposed a workaround, which then became this PR.

I'm happy to keep rebasing my branch but I don't wish to bother folke about it.

Copy link
Contributor

github-actions bot commented Jul 6, 2024

This PR is stale because it has been open 60 days with no activity.

@LunarLambda
Copy link
Contributor Author

Apparently the linked issues will be closed by github actions soon due to being open for so long, as I just received multiple emails about it.

@folke
Copy link
Owner

folke commented Jul 6, 2024

Fixed by #180

@folke folke closed this Jul 6, 2024
folke pushed a commit that referenced this pull request Jul 6, 2024
## What is this PR for?
When using a pattern like `[[(KEYWORDS)\s*(\([^\)]*\)?:)]]` to include
the colon character into the capture group, it doesn't take it into
consideration for highlighting because currently the range is up to
`start + #matched - 1`. This changes the range to `start + #matched`, so
it can match correctly until the end of the matched capture group.

Inspiration was taken from #255, but since #180 was preferred over it,
at least make it possible so that it highlights exactly what the users
define.
<!-- Describe the big picture of your changes to communicate to the
maintainers
  why we should accept this pull request. -->

## Does this PR fix an existing issue?
No
<!--
  If this PR fixes any issues, please link to the issue here.
  - Fixes #<issue_number>
-->
@folke folke reopened this Jul 6, 2024
Highlights the first capture group (as before), if there is a second
(typically nested) capture group, use it for keyword matching.

Allows highlighting `TODO(foo)` using `.*<((KEYWORDS)%(\(.{-1,}\))?):`.
@folke folke merged commit 76c8fee into folke:main Jul 6, 2024
8 checks passed
@folke folke mentioned this pull request Jul 6, 2024
3 tasks
folke pushed a commit that referenced this pull request Jul 7, 2024
🤖 I have created a release *beep* *boop*
---


##
[1.3.0](v1.2.0...v1.3.0)
(2024-07-07)


### Features

* added support for fzf-lua
([fe5a7c6](fe5a7c6))
* **fzf:** multiline by default
([8fdea2a](8fdea2a))
* **highlight:** allow extended highlighting patterns
([#185](#185))
([#255](#255))
([76c8fee](76c8fee))
* **highlight:** allow highlighting the full pattern
([#180](#180))
([ad775a7](ad775a7))


### Bug Fixes

* don't use tbl_flatten. Fixes
[#272](#272)
([9c104cf](9c104cf))
* **extension:** provide default icon
([#274](#274))
([7de4e85](7de4e85)),
closes [#202](#202)
* **highlight:** match to the end of length `#matched`
([#288](#288))
([a40fa7e](a40fa7e))
* **telecope:** icons highlight.
([#279](#279))
([4573f4f](4573f4f))
* ternary evaluation in setlist for opts.open
([#252](#252))
([c7a6a02](c7a6a02))
* **trouble:** compatibility with Trouble v3
([#286](#286))
([01b4599](01b4599))


### Reverts

* feat(highlight): allow highlighting the full pattern
([#180](#180))
([996d1a7](996d1a7))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feature: TODO with numbers [Feature Request]: highlight author in TODO(author):
6 participants