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(ui): Tab Renaming #848

Merged
merged 4 commits into from
Jan 22, 2024
Merged

feat(ui): Tab Renaming #848

merged 4 commits into from
Jan 22, 2024

Conversation

muhmud
Copy link
Contributor

@muhmud muhmud commented Jan 4, 2024

  • Ported over changes from previous PR

@muhmud
Copy link
Contributor Author

muhmud commented Jan 5, 2024

Hey @akinsho , I think this is pretty much ready to look at.

Quick question: how do I run the tests?

Copy link
Owner

@akinsho akinsho left a comment

Choose a reason for hiding this comment

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

Looks good in general made a few comments. Re. tests I'll run them whenever you push a change since it needs approval to run

lua/bufferline/commands.lua Outdated Show resolved Hide resolved
lua/bufferline/tabpages.lua Outdated Show resolved Hide resolved
lua/bufferline/tabpages.lua Show resolved Hide resolved
lua/bufferline/commands.lua Show resolved Hide resolved
Copy link
Owner

@akinsho akinsho left a comment

Choose a reason for hiding this comment

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

Almost there just noticed that the rename function was triggering a get but not sure why?

name = string(tabnr)
end
api.nvim_tabpage_set_var(tabnr, "name", name)
M.get()
Copy link
Owner

Choose a reason for hiding this comment

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

I just noticed this before merging? why does this function need to be called? is this trying to force a refresh? There is a ui.refresh function for that if so

Copy link

Choose a reason for hiding this comment

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

I'm assuming my original reasoning was because it was a more tab scoped re-render call? If ui.refresh will work fine, though, then that'd make more sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll make this change & test it out

Copy link
Contributor Author

@muhmud muhmud Jan 22, 2024

Choose a reason for hiding this comment

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

Pushed a commit for this & tested it out; looks like it's working

@muhmud muhmud changed the title Tab Renaming feat(ui): Tab Renaming Jan 22, 2024
Copy link
Owner

@akinsho akinsho left a comment

Choose a reason for hiding this comment

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

LGTM thanks 🚀

@akinsho akinsho merged commit f2e6c86 into akinsho:main Jan 22, 2024
2 checks passed
@muhmud muhmud deleted the tab-renaming branch January 22, 2024 17:28
sstallion pushed a commit to sstallion/bufferline.nvim that referenced this pull request Jul 3, 2024
* ported over changes from previous pr

* Update lua/bufferline/commands.lua

Co-authored-by: Akin <[email protected]>

* Update lua/bufferline/tabpages.lua

Co-authored-by: Akin <[email protected]>

* change to use ui.refresh

---------

Co-authored-by: Muhmud Ahmad <[email protected]>
Co-authored-by: Akin <[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.

None yet

3 participants