feat: Indicate modified buffers #1835

chomosuke · 2022-12-18T03:48:14Z

closes #1577.

New options and their docs are already completed, I'm opened to feedback on those.

TODOs:

alex-courtis

Looking good so far.

OK

defaults

    modified = {
      enable = true,
      show_on_dirs = true,
      show_on_open_dirs = false,
      debounce_delay = 50,
    },

    update_focused_file = {
      enable = true,
      debounce_delay = 15,

Manual refresh

:NvimTreeToggle

:NvimTreeClose :NvimTreeOpen

Change directory to a different sibling then back

NOT OK

does not show on opened directory

    modified = {
      enable = true,
      show_on_dirs = true,
      show_on_open_dirs = false,
      debounce_delay = 50,
    },

does not show on any directory

    modified = {
      enable = true,
      show_on_dirs = true,
      show_on_open_dirs = true,
      debounce_delay = 50,
    },

doc/nvim-tree-lua.txt

lua/nvim-tree/modified.lua

lua/nvim-tree.lua

doc/nvim-tree-lua.txt

Refactored to make everything use HighlightedString to remove all the complex `insert_highlight` calculation. Not tested.

chomosuke · 2022-12-23T05:12:14Z

alex-courtis · 2022-12-29T22:53:32Z

We could add to base16 when we are done nvim-mini/mini.nvim#181

alex-courtis

Looking good, apologies for the delay.

Tested some with the following general cases:

git
diagnostics
symlinks
indent markers

Testing all permutations and combinations is not feasible however sampled the following:

"Failing" but good enough as they are consistent

glyphs.modified = "XY" shows both, missing, same as git and file icons, good enough
icons.modified_placement = "bleh" missing, same as git, good enough

TODO

fix extra folder padding #1835 (comment)

alex-courtis · 2022-12-30T04:37:47Z

doc/nvim-tree-lua.txt

+      modified = {
+        enable = false,
+        show_on_dirs = true,
+        show_on_open_dirs = false,


Could we perhaps change this to be consistent with git?

show_on_dirs = true, show_on_open_dirs = true,

I actually wanted to change git & diagnostics to the same as modified because I think that'd be the default that most people want, but didn't cause i thought that'd be breaking.

Idk what is this plugin's stance on changes in default though, if you think it's alright I can make that change in this PR.

In the mean time I'll change modified to be consistent with git for now :).

Yeah, I'd like to change those defaults, they make more sense. That's what I use.

Users do get upset when we make changes like that so we have avoided doing so.

I am open to trying it again: how about we make a separate PR for those sensible defaults that we can revert if there is upset.

I have this idea in my head for a while now. Wanted to open an issue but didn't find time to do investigation deep enough to justify it.
I would like to discuss future of the project and breaking changes moving forward. I wanted to argue in favour of releasing pre-release version with multiple breaking changes: removed/unified functionality, reorganised configuration options, general clean up. I think project could greatly benefit from refactor without worrying about breaking changes. It would then be released as version 2 for example. Would there be any appetite for that at all?

reorganised configuration options, general clean up

Yes. git.ignore not being in filters. renderer too large. diagnostics.icons not in icons.glyphs. Some root options not in view or their own section.

I would like to discuss future of the project and breaking changes moving forward.

We can break things in a controlled manner that will result in a better experience. We are also good at refactoring options, although we don't use it much.

I wanted to argue in favour of releasing pre-release version with multiple breaking changes

It would then be released as version 2 for example.

Many lua plugins and packer are getting very good at handling versions. I think it is time.

master could remain as "version 1" to avoid interruptions to users, however we could add devel, weekly etc. along with stable, stable-2, stable-2.1, stable-2.1.3 etc. I like SEMVER.

I did discover an interesting pattern with lualine:

User is prompted to run :LualineNotices when there is a configuration problem.

theme(base16): nvim-base16 is not currently present in your runtimepath, make sure it is properly installed, fallback to default colors.

That's great to hear. Let's open a dedicated issue to discuss it further.

https://github.com/nvim-tree/nvim-tree-management

doc/nvim-tree-lua.txt

lua/nvim-tree/renderer/components/padding.lua

doc/nvim-tree-lua.txt

alex-courtis · 2022-12-30T06:15:10Z

lua/nvim-tree/renderer/builder.lua

-  if not git_icons_and_hl_groups then
+---@class HighlightedString
+---@field str string
+---@field hl string|nil


Thanks for starting this documentation pattern. We should use it more.

sumneko_lua gives me a warning however it does that for other lua code. I might look at putting a suppression in.

hmmm interesting, it doesn't give me any warnings. But maybe putting in suppression in .luarc.json is a good idea for this one.

The problem was on my end... I was putting the current directory in the workspace twice.

I've actually managed to remove all the suppressions: e322fbb

alex-courtis

Last one:

All defaults but modified = "[+]",

Error executing vim.schedule lua callback: ...vt-dev/site/pack/packer/start/packer.nvim/lua/packer.lua:359: Vim(lua):E5108: Error executing lua Vim:E239: Invalid sign text: [+]
stack traceback:
        [C]: in function 'sign_define'
        ...start/dev/lua/nvim-tree/renderer/components/modified.lua:19: in function 'setup_signs'
        ...start/dev/lua/nvim-tree/renderer/components/modified.lua:36: in function 'setup'
        ...te/pack/packer/start/dev/lua/nvim-tree/renderer/init.lua:113: in function 'setup'
        /tmp/nvt-dev/site/pack/packer/start/dev/lua/nvim-tree.lua:828: in function 'setup'
        /home/alex/.local/share/nvim/nvt-dev.lua:34: in function 'setup'
        [string ":lua"]:1: in main chunk
        [C]: in function 'cmd'
        ...vt-dev/site/pack/packer/start/packer.nvim/lua/packer.lua:359: in function ''
        vim/_editor.lua: in function <vim/_editor.lua:0>
stack traceback:
        [C]: in function 'cmd'
        ...vt-dev/site/pack/packer/start/packer.nvim/lua/packer.lua:359: in function ''
        vim/_editor.lua: in function <vim/_editor.lua:0>

Happens for git and diagnostics too.

Proposal:

Only call vim.fn.sign_define if the user has specified "signcolumn"
Trim the glyph to 2 characters

chomosuke · 2022-12-31T03:57:59Z

@alex-courtis Sadly, because lua dose not do utf8 string properly. We can't determine the length of a string or trim a string without some dependency such as https://github.com/uga-rosa/utf8.nvim. I've also tried to pass the string via nvim_eval to vim script and trim it there but had no luck.

…column

alex-courtis · 2022-12-31T04:17:16Z

@alex-courtis Sadly, because lua dose not do utf8 string properly. We can't determine the length of a string or trim a string without some dependency such as https://github.com/uga-rosa/utf8.nvim. I've also tried to pass the string via nvim_eval to vim script and trim it there but had no luck.

In that case, we will have to go with the first option:

Only call vim.fn.sign_define if the user has specified "signcolumn"

It is failing if you specify many characters even when modified.enabled = false

I really want to be able to use [+] "after" ;)

chomosuke · 2022-12-31T04:19:41Z

OH YEAH sure, I forgot that option existed after trying so hard to do the second one haha.

alex-courtis · 2022-12-31T04:46:25Z

The note is not enough...

Try running with all defaults and glyphs.modified = "XXX"

chomosuke · 2022-12-31T04:48:10Z

whoops sorry, forgot to push.

alex-courtis · 2022-12-31T04:52:22Z

awesome

    .hooks
    doc [+]
    lua
      nvim-tree
       nvim-tree.lua [+]
    scripts

alex-courtis

I will enjoy this.

chomosuke added 4 commits December 17, 2022 14:41

Outlined new options

b65ecd6

Merge branch 'master' into feat-modified-buffers

b14b135

highlight_modified is highlight_opened_files

ece4c37

prototype with autocmd

a34c907

alex-courtis mentioned this pull request Dec 18, 2022

General View Debounce #1836

Closed

alex-courtis requested changes Dec 18, 2022

View reviewed changes

doc/nvim-tree-lua.txt Show resolved Hide resolved

doc/nvim-tree-lua.txt Outdated Show resolved Hide resolved

doc/nvim-tree-lua.txt Outdated Show resolved Hide resolved

doc/nvim-tree-lua.txt Show resolved Hide resolved

chomosuke added 5 commits December 19, 2022 19:24

Merge branch 'master' into feat-modified-buffers

84e4ff5

moved modified into glyphs

e501f16

show_on_dirs and show_on_open_dirs

00d3739

icon placement before & after

2ad6125

_get_filename_offset

b3ed7c9

chomosuke force-pushed the feat-modified-buffers branch from f3573d2 to b3ed7c9 Compare December 22, 2022 01:02

fixed :wq doesn't update modified indicator

b4ef5e0

alex-courtis reviewed Dec 23, 2022

View reviewed changes

lua/nvim-tree/modified.lua Outdated Show resolved Hide resolved

alex-courtis requested changes Dec 23, 2022

View reviewed changes

lua/nvim-tree.lua Outdated Show resolved Hide resolved

doc/nvim-tree-lua.txt Outdated Show resolved Hide resolved

chomosuke added 3 commits December 23, 2022 17:19

highlight_modified, signcolumn modified_placement

e95d3a5

Refactored to make everything use HighlightedString to remove all the complex `insert_highlight` calculation. Not tested.

updated doc to match the reality of no multi char for glyphs.modified

9b04ff5

fixed git signcolumn doesn't show

8bf609c

chomosuke added 3 commits December 23, 2022 19:21

fixed highlight_modified gets replaced by highlight_opened_files

c7b20ca

fixed renderer.icons.show.modified = false crash

621d634

updated doc to reflect empty icon not breaking rendering

2bbccb1

chomosuke force-pushed the feat-modified-buffers branch from dfcbab7 to 845069d Compare December 27, 2022 23:05

removed debounce_delay to implement in a later PR

845069d

chomosuke marked this pull request as ready for review December 27, 2022 23:06

chomosuke requested a review from alex-courtis December 27, 2022 23:07

chomosuke mentioned this pull request Dec 27, 2022

Performance issue when working with telescope #1844

Closed

chomosuke requested a review from gegoune December 28, 2022 23:46

doc nit: order placement

efb3805

alex-courtis requested changes Dec 30, 2022

View reviewed changes

chomosuke added 3 commits December 31, 2022 13:02

change modified dirs default to be consistent with git

ad6cb10

illegal git & modified placement changed to default

061a154

don't assume icon exist

38846ab

chomosuke requested a review from alex-courtis December 31, 2022 00:27

nit remove comment

9e15b32

alex-courtis requested changes Dec 31, 2022

View reviewed changes

Merge branch 'master' into feat-modified-buffers

e5c104d

Noted in doc that glyphs can't have more than 2 characters if in sign…

4554681

…column

chomosuke requested review from alex-courtis and removed request for gegoune December 31, 2022 04:43

Don't sign_define if placement isn't signcolumn

413dc30

alex-courtis approved these changes Dec 31, 2022

View reviewed changes

alex-courtis merged commit dcc344c into master Dec 31, 2022

chomosuke deleted the feat-modified-buffers branch December 31, 2022 04:54

Uh oh!

feat: Indicate modified buffers #1835

feat: Indicate modified buffers #1835

Uh oh!

Conversation

chomosuke commented Dec 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alex-courtis left a comment

Choose a reason for hiding this comment

OK

NOT OK

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chomosuke commented Dec 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alex-courtis commented Dec 29, 2022

Uh oh!

alex-courtis left a comment • edited by chomosuke Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alex-courtis Dec 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alex-courtis left a comment

Choose a reason for hiding this comment

Uh oh!

chomosuke commented Dec 31, 2022

Uh oh!

alex-courtis commented Dec 31, 2022

Uh oh!

chomosuke commented Dec 31, 2022

Uh oh!

alex-courtis commented Dec 31, 2022

Uh oh!

chomosuke commented Dec 31, 2022

Uh oh!

alex-courtis commented Dec 31, 2022

Uh oh!

alex-courtis left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

chomosuke commented Dec 18, 2022 •

edited

Loading

chomosuke commented Dec 23, 2022 •

edited

Loading

alex-courtis left a comment •

edited by chomosuke

Loading

alex-courtis Dec 31, 2022 •

edited

Loading