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: highlighting of path coordinates #3153

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

alycklama
Copy link
Contributor

Description

Adds highlighting of path coordinates by using the existing highlight group TelescopeResultsLineNr.

Type of change

  • New feature (non-breaking change which adds functionality)

Screenshots of affected windows

Live Grep

image

Grep Word

image

Buffers

image

LSP definitions

image

LSP references

image

LSP type definitions

image

Jumplist

image

Loclist

image

How Has This Been Tested?

This has been tested manually (see screenshots)

  • Live Grep
  • Grep Word
  • Buffers
  • LSP definitions
  • LSP references
  • Jumplist
  • Loclist

Configuration:

  • Neovim version (nvim --version):

NVIM v0.10.0
Build type: Release
LuaJIT 2.1.1716656478

  • Operating system and version:

Sonoma 14.4.1

Checklist:

  • My code follows the style guidelines of this project (stylua)
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (lua annotations)

@alycklama alycklama changed the title Feature: highlighting of path coordinates feat: highlighting of path coordinates Jun 2, 2024
@jamestrew
Copy link
Contributor

I'm actually planning to move the line+col numbers over next to the file path. I wasn't planning on the current behavior being a permanent thing. See #3156

I'm thinking of adding a wrapper around transform_path that handles concatenating the line/col numbers. This would consolidate all the manual handling of line/col number handling across all the entry makers and allow a more consistent disable_coordinates option across all pickers that offer line/col numbers in the entries.

I want to give Conni a chance to approve this before making these moves. #3152

@alycklama
Copy link
Contributor Author

I'm actually planning to move the line+col numbers over next to the file path. I wasn't planning on the current behavior being a permanent thing. See #3156

I'm thinking of adding a wrapper around transform_path that handles concatenating the line/col numbers. This would consolidate all the manual handling of line/col number handling across all the entry makers and allow a more consistent disable_coordinates option across all pickers that offer line/col numbers in the entries.

I agree. One thing to add, not related to this change.

In retrospective, I believe that "filename_first" should not be a path_transform option. Conceptually, it's no longer a path once you put the filename in front (e.g. hello-world.txt /tmp). Ideally, all options to transform a path should still return a path. How to display the path could then become a new option outside of transform_path. The advantage of this approach is that you can compose any path transformer with any other path transformer. As they all operate on a path, the user can decide how to compose the options sequentially. The reverse_directories option part of filename_first can become a new path_transform option: reverse. Showing or hiding the line numbers can then also be part of the function that decides how to render the path (filename in front and/or with line numbers).

I believe it would simplify things a bit.

@Conni2461
Copy link
Member

I'm thinking of adding a wrapper around transform_path that handles concatenating the line/col numbers.

Would make sense. But this should probably be a new interface that lives alongside the only one, transform_path_lnumcol

In retrospective, I believe that "filename_first" should not be a path_transform option.

I disagree. Transform path was never required to return a valid path, maybe the name is badly choosen. but it always was a way to take a path + config and return a modified version of that, shortened, or only tail, or absolute, or with filename first. And we also have a way for users to modify the path as they want.

Intoducing a new function wouldnt change that at all, we still need that ONE function (currently transform_path) for our or extension entry makers to use. If you want you can introduce a new function that actually returns a valid path, but this will be a local function (i dont see a reason why we need to expose something like that), if you feel like that improves readability

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.

3 participants