Added smart_path option for file name getters. #1

qualious · 2021-05-24T11:22:41Z

tom-anders/telescope-vim-bookmarks.nvim#2

Hey!

So I've done this after the discussion with @Conni2461 and how this would overall benefit the project.

Placing myself at mercy, I've also decided to change the enum logic as well. So that a user doesn't have to deal with importing/exporting/creating enums but configure the options as it is (ie. smart_path = true)

Just to have the preview here as well:

Let me know what you think!

caojoshua

Before I look too deep into code, can you explain what exactly is a smart path? From your examples in File paths are relatively shown depending on other files tom-anders/telescope-vim-bookmarks.nvim#2, it looks like smart relative paths. Could we come up with a more specific name?
Still not exactly sure how we are going to do enums, but TJ just responded in Consistent filepath display and code cleanup. nvim-telescope/telescope.nvim#839, so I'm gonna let that play out first
Since I still don't fully understand, depending on how complicated this is, I might just let the current PR merge first, and then we can create this as a separate PR

qualious · 2021-05-25T11:46:45Z

I think the naming is reasonable. as you said, it's smart relative paths that are calculated based on other files names in the context. I wanted to keep the <>_path naming convention.
Well, this was just a suggestion. Continue as you were
I can comment out the code line by line here to make it more understandable. But basically it saves all files within the context and for each new file function looks at the difference between saved filepaths and current file. then according to their difference, it takes the biggest difference it can take (as in if a/b/c/d, a/b/d differs at index 1) and then adjusts the filenames according to the biggest index it can find, taking the one before it to give context. (../b/c/d/, ../b/d)

I thought it should be a separate implementation as well but I was directed to this by one of the main contributors - which makes it safe to merge in my mind. The function itself can benefit from open source community as well.

caojoshua · 2021-05-26T06:20:46Z

I updated my pull request in the main repo to not use enums: nvim-telescope#839

I think these changes would benefit Telescope, but I think these should be merged as a separate PR in the main repo in favor of smaller PRs. @Conni2461 would you prefer combining into one PR, or try to merge this separately after the current PR is merged? I can work with either way.

Conni2461 · 2021-05-31T10:40:29Z

I don't really prefer one over the other. I am fine with both, tbh.

Note: we are squashing PRs so if you want full commit credit you probably want to make a separate PR to telescope :)

qualious · 2021-05-31T18:44:09Z

Hey!

Wanted to say I don’t really care about that when it’s improving the product I’m using day to day. Thanks for y’all’s effort.

I can do the PR creation after this PR is merged if the owner prefers that way because I feel like that’s the case. Also maybe I can improve it in the meanwhile.

I’ve been using it locally for a while now and it’s working accordingly. there is a bug that happens every once in a while - the second file in the results shows up as full path sometimes when there are 4+ results. (i’m suspecting that there is a bug with initial loop logic) but if you hover over it, it corrects itself (i’m assuming because of rerender).

i can make the logic more solid to prevent such a thing and make it “production ready”. but we are going to go in a release cycle at work in a month or so — but i can make time here and there.

if you guys can give me an approximate merge time that’d be perfect for my planning.

also if things comes down to it @Conni2461 you can copy my code and improve it and push to the main repo. you have my explicit permission.

Conni2461 · 2021-05-31T19:03:51Z

lua/telescope/utils.lua

@@ -147,6 +148,83 @@ utils.path_tail = (function()
  end
 end)()

+utils.slice = function(tbl, s, e)
+  local pos, new = 1, {}


Do you actually want to copy all elements between s and e into a new table? Thats not great.

Maybe this plenary module can help https://github.com/nvim-lua/plenary.nvim/blob/master/lua/plenary/iterators.lua
No idea how it works tho. But i saw a range function. This is the spec file https://github.com/nvim-lua/plenary.nvim/blob/master/tests/plenary/iterators_spec.lua

Conni2461 · 2021-05-31T19:04:08Z

lua/telescope/utils.lua

+
+
+
+PATHS = {}


This shouldn't be a global right?

Conni2461 · 2021-05-31T19:05:57Z

lua/telescope/utils.lua

+    final = utils.slice(initial_path, length - 1, length)
+    final = table.concat(final, "/")


If you are only using slice for create a string afterwards you should look for a different method.

yeah i figured that but i was like “i’m already using it 🤷🏻‍♀️“.

it’s a quick hack, not production ready at all. i’ll fix these if i’m going to open the pr on the main repo. i appreciate the feedback

edit: and i honestly wasn’t hopeful about the initial PR. i didn’t know we would get you involved

caojoshua · 2021-06-01T05:12:57Z

Hey!

Wanted to say I don’t really care about that when it’s improving the product I’m using day to day. Thanks for y’all’s effort.

I can do the PR creation after this PR is merged if the owner prefers that way because I feel like that’s the case. Also maybe I can improve it in the meanwhile.

I’ve been using it locally for a while now and it’s working accordingly. there is a bug that happens every once in a while - the second file in the results shows up as full path sometimes when there are 4+ results. (i’m suspecting that there is a bug with initial loop logic) but if you hover over it, it corrects itself (i’m assuming because of rerender).

i can make the logic more solid to prevent such a thing and make it “production ready”. but we are going to go in a release cycle at work in a month or so — but i can make time here and there.

if you guys can give me an approximate merge time that’d be perfect for my planning.

also if things comes down to it @Conni2461 you can copy my code and improve it and push to the main repo. you have my explicit permission.

Maybe can merge in a week-ish.

I've linked to this pull request as a TODO from this Telescope wiki, to make sure this is on our radars.

menash134 · 2021-08-27T11:38:40Z

Hi, I also looking for smart file paths. My idea would be naive and simple. could you just allow me to configure the depths of the file path to be displayed? This would be ideal for my usecase. may be before the depth it could be "hidden/shorten"!!!

caojoshua · 2021-08-28T04:35:49Z

Hey, this is my fork that I use for my own pull requests. You might want to check nvim-telescope#914, although seems that pull request is not active.

caojoshua and others added 3 commits May 12, 2021 22:47

Consistent file path display and code cleanup.

0048228

path display options as enum.

c885f9e

Added smart_path option for file name getters.

efd5dda

qualious mentioned this pull request May 24, 2021

File paths are relatively shown depending on other files tom-anders/telescope-vim-bookmarks.nvim#2

Closed

caojoshua reviewed May 25, 2021

View reviewed changes

caojoshua force-pushed the filepath branch from 0b20af5 to d1f3d5a Compare May 26, 2021 06:11

Conni2461 reviewed May 31, 2021

View reviewed changes

lua/telescope/utils.lua

PATHS = {}

Copy link

Conni2461 May 31, 2021

Choose a reason for hiding this comment

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

This shouldn't be a global right?

qualious reacted with thumbs up emoji

Conni2461 reviewed May 31, 2021

View reviewed changes

caojoshua force-pushed the filepath branch from d1f3d5a to 87b9b27 Compare June 1, 2021 04:47

caojoshua force-pushed the filepath branch from 87b9b27 to e847780 Compare June 6, 2021 18:05

qualious mentioned this pull request Jun 14, 2021

feat: smart path nvim-telescope/telescope.nvim#914

Merged

caojoshua force-pushed the filepath branch from ac5a0cb to 6d96739 Compare June 15, 2021 05:57

caojoshua force-pushed the filepath branch from 661c15c to e6adacd Compare July 7, 2021 04:41

abzcoding mentioned this pull request Jul 9, 2021

lv-telescope path_display should be smart LunarVim/LunarVim#812

Closed

abzcoding mentioned this pull request Jul 17, 2021

[Feature] Better path_display default for telescope LunarVim/LunarVim#1009

Merged

caojoshua closed this Aug 28, 2021

phortonssf mentioned this pull request Dec 21, 2022

nvim-telescope/community - Gitter phortonssf/.dotfiles#12

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added smart_path option for file name getters. #1

Added smart_path option for file name getters. #1

qualious commented May 24, 2021

caojoshua left a comment

qualious commented May 25, 2021

caojoshua commented May 26, 2021

Conni2461 commented May 31, 2021

qualious commented May 31, 2021

Conni2461 May 31, 2021

Conni2461 May 31, 2021

Conni2461 May 31, 2021

qualious May 31, 2021 •

edited

Loading

caojoshua commented Jun 1, 2021

menash134 commented Aug 27, 2021

caojoshua commented Aug 28, 2021

		final = utils.slice(initial_path, length - 1, length)
		final = table.concat(final, "/")

Added smart_path option for file name getters. #1

Added smart_path option for file name getters. #1

Conversation

qualious commented May 24, 2021

caojoshua left a comment

Choose a reason for hiding this comment

qualious commented May 25, 2021

caojoshua commented May 26, 2021

Conni2461 commented May 31, 2021

qualious commented May 31, 2021

Conni2461 May 31, 2021

Choose a reason for hiding this comment

Conni2461 May 31, 2021

Choose a reason for hiding this comment

Conni2461 May 31, 2021

Choose a reason for hiding this comment

qualious May 31, 2021 • edited Loading

Choose a reason for hiding this comment

caojoshua commented Jun 1, 2021

menash134 commented Aug 27, 2021

caojoshua commented Aug 28, 2021

qualious May 31, 2021 •

edited

Loading