Skip to content

feat: add icons to git_status finder#401

Merged
tjdevries merged 7 commits intonvim-telescope:masterfrom
sunjon:git_icons
Apr 9, 2021
Merged

feat: add icons to git_status finder#401
tjdevries merged 7 commits intonvim-telescope:masterfrom
sunjon:git_icons

Conversation

@sunjon
Copy link
Copy Markdown
Contributor

@sunjon sunjon commented Jan 6, 2021

This is WIP as there's an issue with some of the double width icons getting cut in half.

giticons

Can anyone test and see if they see the same?

@sunjon sunjon requested a review from cempassi January 6, 2021 17:46
Comment thread lua/telescope/make_entry.lua Outdated
Copy link
Copy Markdown
Contributor

@cempassi cempassi left a comment

Choose a reason for hiding this comment

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

LGTM!

@sunjon
Copy link
Copy Markdown
Contributor Author

sunjon commented Jan 6, 2021

LGTM!

alright, I'll assume the icon problems are on my end until more people have tested it.
edit: adding an extra space fixed things a little bit more.. but still quite broken (on kitty).

@sunjon
Copy link
Copy Markdown
Contributor Author

sunjon commented Jan 6, 2021

The default icon choices are a bit random.
I'm also not sure if each X/ Y column should be treated individually, or if their state should be represented by the combination of them both (if that makes sense).

@sunjon sunjon requested a review from Conni2461 January 6, 2021 18:50
@sunjon
Copy link
Copy Markdown
Contributor Author

sunjon commented Jan 6, 2021

these are the options available if anyone else has time to test;

opts.git_icons = {
  added      = "  ",
  changed    = "  ",
  copied     = "  ",
  deleted    = "  ",
  untracked  = "  ",
  unmerged   = "  ",
}

@Conni2461
Copy link
Copy Markdown
Member

Hmm i don't know how i feel about this. I used git status --short on cli for years and i got so used to them now that i need my M and A and so 😆 I know i can change them now but i think the default should resemble git status --short or git status --porcelain as close as possible.
Also having left and right the same default color is kinda weird. I sometimes stage things what i have done but i still have to do more work so i eventually will end up with MM and both having a different color. Thats the whole point of splitting them into two parameters.

I think that message is a mess. I am still to tired.

@sunjon
Copy link
Copy Markdown
Contributor Author

sunjon commented Jan 7, 2021

Hmm i don't know how i feel about this. I used git status --short on cli for years and i got so used to them now that i need my M and A and so laughing I know i can change them now but i think the default should resemble git status --short or git status --porcelain as close as possible.
Also having left and right the same default color is kinda weird. I sometimes stage things what i have done but i still have to do more work so i eventually will end up with MM and both having a different color. Thats the whole point of splitting them into two parameters.

I think that message is a mess. I am still to tired.

I agree. I wanted someone else's opinion as to how to make the display coherent.
I'm open to:

  • ) leaving everything as it is.
  • ) adding some color to the existing displayer
  • ) find a way to make icons make sense to people used to the original format.

edit: also I just wanted to put it up here so I could get some feedback on my/the icon width issue! :)

@Conni2461
Copy link
Copy Markdown
Member

Conni2461 commented Jan 7, 2021

Its all good :) Still thanks for putting it up. Its important that we talk about things like this and find a common ground and ultimately have a better builtin :)

Right now i am thinking having a second entry_maker which can be enabled with an opts and that one can have icons. You can use only one row and so on. Because i think it will be very hard to realize that all in one entry_maker.

@elianiva
Copy link
Copy Markdown
Member

elianiva commented Jan 9, 2021

are you going to make the git_icons option configurable via setup{} as well? Everything seems to be working and I didn't get that cut out icons

@cempassi cempassi closed this Jan 9, 2021
@cempassi cempassi reopened this Jan 9, 2021
Comment thread lua/telescope/make_entry.lua Outdated
local make_display = function(entry)
local modified = "TelescopeResultsDiffChange"
local staged = "TelescopeResultsDiffAdd"
local icons = opts.git_icons or git_icon_defaults
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should probably do something where we just create a new icons table that has a merging of the passed in icons and the defaults, if you only want to override one thing.

Also, maybe a list of the names somewhere and/or an example with pretty icons.

@tjdevries tjdevries merged commit 5bd6f5c into nvim-telescope:master Apr 9, 2021
@tjdevries
Copy link
Copy Markdown
Member

If anyone doesn't like the new setup, let's chat in a new issue / PR. I think this is cool :)

williamboman added a commit to williamboman/telescope.nvim that referenced this pull request Apr 14, 2021
* upstream/master:
  picker(live_grep): add option to grep only over open files (nvim-telescope#666)
  git(action): create and checkout branch (nvim-telescope#755)
  readme: fix broken links and spelling errors (nvim-telescope#753)
  added a new DynamicFinder (which can be used with rust_analyzer) (nvim-telescope#705)
  feat: add icons to git_status finder (nvim-telescope#401)
  fix: update to newer code (nvim-telescope#744)
  pickers(buffers): added only_cwd opt (nvim-telescope#739)
  feat: asyncify pickers - except for live_grep (nvim-telescope#709)
  fix: Use standardized names for current buffer fuzzy find (nvim-telescope#737)
  fix(git_branches): use the quoted fields instead of json-formatting and fix regressions with nvim-telescope#695 (nvim-telescope#704)
  feat: buf highlights for current buffer fuzzy find (nvim-telescope#732)
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.

5 participants