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: List toggling/inverting for sublists #1492

Merged
merged 1 commit into from
Jul 24, 2024

Conversation

mtrajano
Copy link
Contributor

First pass. Might still need some cleaning up but toggling / inverting sublists is working. I can also write some tests for this, what is the status on testing? Do we have this set up already?

Demo:

Screen.Recording.2024-06-28.at.3.45.13.PM.mov

---@param match_self boolean #If true can match on self, if false needs to match on ancestor to node
---@return TSNode?
-- TODO: if remove match_self param, need to check other functionality that depends on this
find_parent = function(node, types, match_self)
Copy link
Contributor Author

@mtrajano mtrajano Jun 28, 2024

Choose a reason for hiding this comment

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

Not sure if this was intended functionality but the previous version of this method allowed to match on the node passed so I had to include this param to stricly match on ancestor. The default is to keep the previous behavior (match_self = true).

node = node:parent()
end

if type(types) == "string" then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Simplified the method so we're only working with one type of param (table) otherwise should behave the same way. For tables it also used to check for equality whereas now it does pattern matching. If a string is passed that is not a pattern it should behave the same way

---@param types table|string #If `types` is a table, this function will attempt to match any of the types present in the table.
-- If the type is a string, the function will attempt to pattern match the `types` value with the node type.
find_parent = function(node, types)
local _node = node
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not really sure what this local _node was doing so I removed it. Also added the TSNode typehint to get rid of the ignore diagnostic comments cc: @pysan3

local parent = module.required["core.integrations.treesitter"].find_parent(node, {
"^unordered_list%d$",
"^ordered_list%d$",
"^generic_list$", -- for level 1 lists the parent is a generic_list
Copy link
Contributor Author

Choose a reason for hiding this comment

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

because find_parent can now accept patterns we can now change this to also be a list of pattern instead of an exhaustive list

@vhyrro
Copy link
Member

vhyrro commented Jul 8, 2024

Great work, unfortunately the refactor of the find_parent function seems to have broken quite a lot of functionality that relied on it. An example is link resolution: pressing on a simple link will cause the link resolver to fail.

I don't think I have the time to track down the source of the issue for this release, but I can sure come back to this and we can try to figure it out :)

@mtrajano
Copy link
Contributor Author

Hey @vhyrro thank you for the heads up I can look into it and clean this pr up a bit. I'll ping her once again once I get that out.

@mtrajano mtrajano force-pushed the toggle-lists-fix branch 2 times, most recently from 6374d45 to aeb286b Compare July 23, 2024 14:15
@mtrajano mtrajano changed the title WIP: working toggling for sublists feat: List toggling/inverting for sublists Jul 23, 2024
@mtrajano
Copy link
Contributor Author

@vhyrro updated the previous pr. Essentially kept the scope of changes to just the pivot module to prevent the other breaking changes. Open to any feedback/improvements you may have

@vhyrro
Copy link
Member

vhyrro commented Jul 24, 2024

Fantstic, thank you! I've been unable to break it in my testing and it works much better than the current implementation :)

@vhyrro vhyrro merged commit 169495c into nvim-neorg:main Jul 24, 2024
5 of 6 checks passed
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.

2 participants