Skip to content

Conversation

@wayjam
Copy link
Contributor

@wayjam wayjam commented Dec 14, 2020

TestCase: https://github.com/go-kit/kit/blob/3796a6b25f/sd/etcdv3/client.go

Path: sd/etcdv3/client.go
NodePath: sd/etcdv3 and sd/etcd will be both expanded.

@vercel
Copy link

vercel bot commented Dec 14, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/enix/gitako/8z13yjj42
✅ Preview: https://gitako-git-fix-expand-path.enix.now.sh

@EnixCoda
Copy link
Owner

Whoa, thank you for finding this bug, nice catch! @wayjam

Based on your work, I found another possible fix to the bug. If it works too, it should be faster to run and easier to understand.

Here it is.

diff --git a/src/utils/VisibleNodesGenerator.ts b/src/utils/VisibleNodesGenerator.ts
index c869319..52f8015 100644
--- a/src/utils/VisibleNodesGenerator.ts
+++ b/src/utils/VisibleNodesGenerator.ts
@@ -287,19 +287,16 @@ class FlattenLayer extends CompressLayer {
     const rootNode = this.compressedRoot
     if (rootNode) {
       await traverse(
-        [rootNode],
+        rootNode.contents,
         async node => {
-          const overflowChar = node.path[path.length + 1]
+          const overflowChar = path[node.path.length]
           const match = path.startsWith(node.path) && (overflowChar === '/' || !overflowChar)
-          if (node.path) {
-            // rootNode.path === ''
           if (match) {
             if (node.path === path) {
               // do not wait for expansion for the exact node as that will block "jumping from search"
               this.$setExpand(node, true)
             } else await this.$setExpand(node, true)
           }
-          }
           return match
         },
         node => node?.contents || [],

Does it look good to you? If so, would you mind me committing it after your commit?

@wayjam
Copy link
Contributor Author

wayjam commented Dec 16, 2020

@EnixCoda That's find with me.

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