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

ref: local mlocal groups #446

Merged
merged 2 commits into from
Apr 10, 2023
Merged

ref: local mlocal groups #446

merged 2 commits into from
Apr 10, 2023

Conversation

Iron-E
Copy link
Collaborator

@Iron-E Iron-E commented Apr 7, 2023

This PR has a few advantages:

  1. Uses the naming scheme seen in other modules
    • i.e. local mlocal groups
  2. Creates a clear naming difference between group_clumps and group:
    old name new name
    barbar.render.group barbar.groups.item
    barbar.render.group[] barbar.groups
    barbar.render.group_clump barbar.render.group
  3. Alleviates cyclical resolution of @params in the groups module
    • e.g. almost all of the params were barbar.render.group[], but barbar.render actually depends on barbar.groups, so it would be cyclical in a traditional type checking system
  4. Performance increase by using for i = x, y, -1 instead of utils.list_reverse

@Iron-E Iron-E added documentation Improvements or additions to documentation refactor Misc. change in an existing component of barbar.nvim labels Apr 7, 2023
@Iron-E Iron-E self-assigned this Apr 7, 2023
@Iron-E Iron-E requested a review from romgrk April 9, 2023 04:23
@Iron-E Iron-E added this to the 1.6 milestone Apr 9, 2023
lua/barbar/groups.lua Outdated Show resolved Hide resolved
lua/barbar/groups.lua Outdated Show resolved Hide resolved
@Iron-E Iron-E force-pushed the ref/groups-module-rename branch from 6a7b983 to bc33c5a Compare April 9, 2023 21:25
@Iron-E
Copy link
Collaborator Author

Iron-E commented Apr 9, 2023

Ok, I've revised the PR based on your feedback. Summary:

  • Clarified group / group_clump:
    old name new name
    barbar.render.group barbar.ui.node
    barbar.render.group_clump barbar.ui.container
  • We have a few modules that work closely together in order to draw the barbar. I've moved them to a new sub-module called barbar.ui (e.g. barbar.renderbarbar.ui.render).

@Iron-E Iron-E force-pushed the ref/groups-module-rename branch from bc33c5a to 2ae209a Compare April 9, 2023 21:39
@Iron-E Iron-E mentioned this pull request Apr 9, 2023
@Iron-E Iron-E force-pushed the ref/groups-module-rename branch 3 times, most recently from de928aa to 47366b7 Compare April 10, 2023 04:00
Also creates a clear naming distinction between `group_clumps` and
`groups`.
@Iron-E Iron-E force-pushed the ref/groups-module-rename branch from 47366b7 to 49897ea Compare April 10, 2023 04:09
lua/barbar/events.lua Outdated Show resolved Hide resolved
lua/barbar/ui/render.lua Outdated Show resolved Hide resolved
@romgrk romgrk merged commit 5dac605 into master Apr 10, 2023
@romgrk romgrk deleted the ref/groups-module-rename branch April 10, 2023 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation refactor Misc. change in an existing component of barbar.nvim
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants