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: Adds syntax highlighting in the telescope picker #386

Merged
merged 6 commits into from
Jul 16, 2024
Merged

feat: Adds syntax highlighting in the telescope picker #386

merged 6 commits into from
Jul 16, 2024

Conversation

dzirtusss
Copy link
Contributor

Adds syntax highlighting in the telescope picker. Issue #384

  1. syntax highlighting is shown in the telescope picker. By Treesitter, so all supported languages work.
  2. adds only_lines picker config to show only code w/o symbols.
  3. fixes several minor bugs where override configs were not applied correctly.

Some results:
image

image image

with only_lines=true

image

Other notes:

  • only_lines - tried to name it in existing naming syntax, no any preferences, up to you
  • this PR doesn't add syntax highlighting everywhere, which is a bigger task
  • this PR doesn't solve all config issues, just some patches. Basically, IMO, all telescope configs need to be refactored to correctly pick: 1) aerial defaults 2) telescope defaults 3) plugin opitions 4) cmd options. This is a different task as well.

@github-actions github-actions bot requested a review from stevearc July 10, 2024 10:00
Copy link
Owner

@stevearc stevearc left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Left a few comments

Comment on lines 62 to 63
local ts_parsers = require("nvim-treesitter.parsers")
local ts_query = require("nvim-treesitter.query")
Copy link
Owner

Choose a reason for hiding this comment

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

This introduces a hard dependency on nvim-treesitter. Could this be reworked to only use the built-in vim.treesitter APIs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, works as well w/o it, done

Comment on lines 87 to 110
for _, value in ipairs(buf_highlights) do
local start_row, start_col, end_row, end_col = unpack(value.range)

if start_row == row then
local type = value.capture:gsub("%..*", "") -- strip subtypes after dot
table.insert(highlights, { { start_col + offset, end_col + offset }, type })
end
end
Copy link
Owner

Choose a reason for hiding this comment

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

This looks like an n-squared algorithm; I would expect it to lag on large files. Could we instead store the buf_highlights in a structure that is indexed by the start_row?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, updated to index based way

Comment on lines 8 to 9
only_lines = false,
show_lines = true,
Copy link
Owner

Choose a reason for hiding this comment

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

This introduces the possibility for a nonsensical configuration (only_lines = true and show_lines = false).

Could we instead have one configuration option show_columns that can be "symbols", "lines", or "both"? And then add a shim so the old show_lines option continues to work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to show_columns way

@github-actions github-actions bot requested a review from stevearc July 15, 2024 20:02
@dzirtusss
Copy link
Contributor Author

@stevearc all updated, please review

@stevearc
Copy link
Owner

LGTM, thanks for the PR!

@stevearc stevearc merged commit 4e77964 into stevearc:master Jul 16, 2024
8 checks passed
emmanueltouzery added a commit to emmanueltouzery/aerial.nvim that referenced this pull request Sep 21, 2024
the highlighting for the symbol type and the symbol name were lost with
that change.
emmanueltouzery added a commit to emmanueltouzery/aerial.nvim that referenced this pull request Sep 21, 2024
the highlighting for the symbol type and the symbol name were lost with
that change.
stevearc added a commit that referenced this pull request Oct 16, 2024
* fix telescope highlights regression from #386

the highlighting for the symbol type and the symbol name were lost with
that change.

* fix: telescope highlighting accounts for multibyte characters

* telescope: allow to configure column widths

---------

Co-authored-by: Steven Arcangeli <[email protected]>
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