Can't `MoveCursorForward` past value in JSON dictionary

[PEZ]

Hi! Thanks for this extension!

Not sure if a feature or bug. (Cursor at |):

{
  "devDependencies": {
    "shadow-cljs": "2.26.7"
  },
  |"dependencies": {
    "snabbdom": "3.5.1"
  }
}

MoveCursorForward

{
  "devDependencies": {
    "shadow-cljs": "2.26.7"
  },
  "dependencies": |{
    "snabbdom": "3.5.1"
  }
}

So far, so good (even if I would have expected the cursor to be at the end of the expression I have jumped pass. Then:

MoveCursorForward

{
  "devDependencies": {
    "shadow-cljs": "2.26.7"
  },
  "dependencies": |{
    "snabbdom": "3.5.1"
  }
}

Whereas I expect:

MoveCursorForward

{
  "devDependencies": {
    "shadow-cljs": "2.26.7"
  },
  "dependencies": {
    "snabbdom": "3.5.1"
  }|
}

MoveCursorBackward

The above is a bit inconsistent with:

{
  "devDependencies": {
    "shadow-cljs": "2.26.7"
  },
  "dependencies": {
    "snabbdom": "3.5.1"
  }|
}

MoveCursorBackward

{
  "devDependencies": {
    "shadow-cljs": "2.26.7"
  },
  |"dependencies": {
    "snabbdom": "3.5.1"
  }
}

While I expect:

{
  "devDependencies": {
    "shadow-cljs": "2.26.7"
  },
  "dependencies": |{
    "snabbdom": "3.5.1"
  }
}

Stuckness backwards too

Further:

{
  "devDependencies": {
    "shadow-cljs": "2.26.7"
  },
  |"dependencies": {
    "snabbdom": "3.5.1"
  }
}

MoveCursorBackward

{
  "devDependencies": {
    "shadow-cljs": "2.26.7"
  },
  |"dependencies": {
    "snabbdom": "3.5.1"
  }
}

While I expect:

{
  "devDependencies": |{
    "shadow-cljs": "2.26.7"
  },
  "dependencies": {
    "snabbdom": "3.5.1"
  }
}

Or possibly:

{
  |"devDependencies": {
    "shadow-cljs": "2.26.7"
  },
  "dependencies": {
    "snabbdom": "3.5.1"
  }
}

[simonacca]

Hi @PEZ , thanks for trying this out!

I agree that the extension should behave according to your intuition in all of your examples, will investigate

[simonacca]

For reference, here is the intuitive behavior encoded as tests (currently failing)

[PEZ]

Very cool! It is quite similar to how we test Calva Paredit stuff.

[simonacca]

Just published pre-release 1.0.20, which should solve the problem of the cursor being stuck. Regarding where the cursor should go after a "move forward", playing around with it I realized that I prefer the behavior where the cursor jumps to the beginning of the next "expression", although I recognize that the classic behavior is to jump to the end of the current expression, so I implemented both.

• taste.MoveCursorForwardToBeginningOfNextNode
• taste.MoveCursorForwardToEndOfNode
• and taste.MoveCursorForward will still behave like taste.MoveCursorForwardToBeginningOfNextNode for compatibility purposes

Let me know how this works for you!

[PEZ]

Thanks!

After some trying I have to admit I do not know how to install pre-releases. 😄

[simonacca]

It was a struggle for me as well 🤪

• uninstall the extension
• reload window
• search for the extension
• click on the down-arrow next to install
• select pre-release

[PEZ]

A word of caution about the separate commands. Two words, actually 😄

1. They get part of your API. People may use them in Joyride scripts or from other extension code.
   • I realize this may be why you kept the compatibility behaviour of the original command, but anyway.
2. Watch out for an explosion.
   • The user may expect there to be a corresponding select command for each movement. In your case you have selectForward, and someone (like me 😄) could start to ask about SelectForwardToEndOfNode.
   • Since there is also backward movement and selection, the commands may need backward counterparts too, for consistency. (Default backward and forward are inconsistent know, btw.)

[PEZ]

Now tried. Found these issues (the first one may not be new):

moveBackward

    {
      "devDependencies": {
        "shadow-cljs": "2.26.7"
      },
      "dependencies": 🫸🏻🫷🏻{
        "snabbdom": "3.5.1"
      }👉🏻👈🏻
    }

So far so, so good. And also:

    {
      "devDependencies": {
        "shadow-cljs": "2.26.7"
      },
      🫸🏻🫷🏻"dependencies": 👉🏻👈🏻{
        "snabbdom": "3.5.1"
      }
    }

But then...:

    {
      🫸🏻🫷🏻"devDependencies": {
        "shadow-cljs": "2.26.7"
      },
      👉🏻👈🏻"dependencies": {
        "snabbdom": "3.5.1"
      }
    }

Where I would expect:

    {
      "devDependencies": 🫸🏻🫷🏻{
        "shadow-cljs": "2.26.7"
      },
      👉🏻👈🏻"dependencies": {
        "snabbdom": "3.5.1"
      }
    }

Move backward and forward

This may be a matter of taste, but I prefer the traditional behaviour. There's the equivalent case for forward. Here's backward:

{
  "devDependencies": 🫸🏻🫷🏻{
    👉🏻👈🏻"shadow-cljs": "2.26.7"
  },
}

Traditional/expected:

{
  "devDependencies": {
    🫸🏻🫷🏻👉🏻👈🏻"shadow-cljs": "2.26.7"
  },
}

I.e. the cursor should get stuck here. In many/most paredit implementations there are separate commands for moving backward/forward up.

[simonacca]

Thanks for testing!

I'm afraid these are the tradeoffs of the current philosophy of tASTe which is to not add language-dependent logic. Meaning that the behavior of each command is determined by the parser for the given language and the command's code (which is the same no matter the language).

Not that I'm opposed to per-language rules on principle, but from a bit of experimentation it looks like adding per-language rules is a huge rabbithole (would have to learn the details of each language, and closely inspect each parsers every time they update) for marginal gains.

Tree sitter's AST

For more detail, please refer to this representation of tree sitter's parse tree, where I'm only showing named nodes. I've also purposefully omitted the tree sitter type (string_content, pair, object) as these are language and context specific.

98168 -> {\n    "devDependencies": {\n      "shadow-cljs": "2.26.7"\n    },\n    "dependencies": {\n      "snabbdom": "3.5.1"\n    }\n  }\n  
  98080 -> {\n    "devDependencies": {\n      "shadow-cljs": "2.26.7"\n    },\n    "dependencies": {\n      "snabbdom": "3.5.1"\n    }\n  }
   97976 -> "devDependencies": {\n      "shadow-cljs": "2.26.7"\n    }
    96368 -> "devDependencies"
     94496 -> devDependencies
    96280 -> {\n      "shadow-cljs": "2.26.7"\n    }
     96184 -> "shadow-cljs": "2.26.7"
      96072 -> "shadow-cljs"
       95192 -> shadow-cljs
      95984 -> "2.26.7"
       95720 -> 2.26.7
   97880 -> "dependencies": {\n      "snabbdom": "3.5.1"\n    }
    97768 -> "dependencies"
     96568 -> dependencies
    97680 -> {\n      "snabbdom": "3.5.1"\n    }
     97584 -> "snabbdom": "3.5.1"
      97472 -> "snabbdom"
       96760 -> snabbdom
      97384 -> "3.5.1"
       97120 -> 3.5.1

Example 1

With this in mind, consider the example where we apply moveForward to:

{
    "devDependencies": 👉🏻👈🏻{
      "shadow-cljs": "2.26.7"
    },
    "dependencies": {
      "snabbdom": "3.5.1"
    }
  }  

The first thing we can do is identify the deepest node containing our selection, in this case 96280.

If we only consier the node's siblings, we are stuck; there is no next node.

Here is where we trade off accuracy for usefulness, by looking for the parent's next sibling (if it has any), then the grandparent's next siblings, and so on.

By doing so, we arrive at:

{
    "devDependencies": 👉🏻👈🏻{
      "shadow-cljs": "2.26.7"
    },
    🫸🏻🫷🏻"dependencies": {
      "snabbdom": "3.5.1"
    }
  }  

Example 2

This is the example of unexpected moveBackward behavior:

  {
      "devDependencies": {
        "shadow-cljs": "2.26.7"
      },
      👉🏻👈🏻"dependencies": {
        "snabbdom": "3.5.1"
      }
    }

The node we start with (deepest node containing selection) is 97768.

This node doesn't have a sibling predecessor, what to do? We must go in the direction of the parent's preceding siblings.

So, similarly to the forward case, this is what we do. if the parent has a preceding sibling, we go to its start, if not, we do the same with the grandparent's preceding sibling, and so forth.

And this is how we end up with

  {
      🫸🏻🫷🏻"devDependencies": {
        "shadow-cljs": "2.26.7"
      },
      👉🏻👈🏻"dependencies": {
        "snabbdom": "3.5.1"
      }
    }

The alternative could be to look into the parent's previous's sibling's children... I've avoided this approach so far because it's unclear when to stop digging down (children's children, and so forth).

Specifically in the example, how do we tell that we should do this:

    {
      "devDependencies": 🫸🏻🫷🏻{
        "shadow-cljs": "2.26.7"
      },
      👉🏻👈🏻"dependencies": {
        "snabbdom": "3.5.1"
      }
    }

and not this:

    {
      "devDependencies": {
        "shadow-cljs": 🫸🏻🫷🏻"2.26.7"
      },
      👉🏻👈🏻"dependencies": {
        "snabbdom": "3.5.1"
      }
    }

[simonacca]

Watch out for an explosion.

I don't mind this so much... I expect this is the other face of the medal when one doesn't provide a very coherent API by adjusting to the specifics of each language

[PEZ]

Totally get the tradeoffs. I would have expected 96368, 96280, 97768, and 97680 to be siblings. If they aren't, then all bets are off. 😄

[simonacca]

Merging and releasing the improvements from https://github.com/simonacca/tASTe/pull/17 for now, happy to revisit this topic when a robust generic solution (or good per-language-rules implementation) comes to light!