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

Handle comma after moving lines #9

Open
zardoy opened this issue Nov 23, 2022 · 5 comments
Open

Handle comma after moving lines #9

zardoy opened this issue Nov 23, 2022 · 5 comments

Comments

@zardoy
Copy link
Owner

zardoy commented Nov 23, 2022

Simple json example:

{
	"foo": 5,
	"bar": true
}

When you move "bar": true above (Move Line Up command, alt+up) this happens:

{
	"bar": true
	"foo": 5,
}

Ideally:

  1. for "bar" key comma should be added
  2. for "foo" key comma should be removed
@zardoy zardoy changed the title Handle comma insertion after moving lines Handle comma after moving lines Nov 23, 2022
@zardoy
Copy link
Owner Author

zardoy commented Nov 23, 2022

For the sake of simplicity:

  1. Let's override alt+up alt+down commands for json & jsonc languages: example
  2. You should jsonc-parser parse probably to determine wether target line is last node

@zardoy
Copy link
Owner Author

zardoy commented Nov 24, 2022

First working basic code:
import { parseTree, findNodeAtOffset, Node } from 'jsonc-parser'

if (!doc) throw new Error('')
if (!editor) throw new Error('')

let swapDirection = -1 as 1 | -1

let root = parseTree(doc.getText())
if (!root) throw new Error('')

// todo do the same for each cursor

let line = doc.lineAt(editor.selection.start)

let node = findNodeAtOffset(root, doc.offsetAt(line.range.start.with(undefined, line.firstNonWhitespaceCharacterIndex)))
if (node?.parent?.type === 'property') node = node.parent
const parent = node?.parent
if (!node || !parent?.children) throw new Error('')
const nodeIndex = parent.children.indexOf(node)

const swapNode = findSiblingNode(parent.children, nodeIndex, swapDirection, line.lineNumber)
if (!swapNode) throw new Error('No swap node')

const isNodeIndexLast = (i: number) => i === parent.children!.length - 1

// todo check line
const swapLine = doc.lineAt(line.lineNumber + swapDirection)
const swapNodeIndex = parent.children.indexOf(swapNode)

const removeTrailingComma = (text: string) => {
    // todo handle comments & trailiing whitespaces
    return text.replace(/,$/, '')
}

const addTrailingComma = (text: string) => {
    // todo handle comments & trailiing whitespaces
    if (text.endsWith(',')) return text
    return `${text},`
}

let curLineText = line.text
let swapLineText = swapLine.text

if (swapDirection === 1 && isNodeIndexLast(swapNodeIndex)) {
    curLineText = removeTrailingComma(curLineText)
    swapLineText = addTrailingComma(swapLineText)
}
if (swapDirection === -1 && isNodeIndexLast(nodeIndex)) {
    curLineText = addTrailingComma(curLineText)
    swapLineText = removeTrailingComma(swapLineText)
}

editor.edit(edit => {
    edit.replace(line.range, swapLineText)
    edit.replace(swapLine.range, curLineText)
})

function findSiblingNode(list: Node[], index: number, swapDirection: 1 | -1, curLineNumber: number) {
    let lastCorrentNode: Node | undefined
    while (true) {
        index += swapDirection
        const nextNode = list[index]
        if (!nextNode) break
        const { line: nodeLine } = doc!.positionAt(nextNode.offset + nextNode.length)
        if (nodeLine === curLineNumber) continue
        if (nodeLine === curLineNumber + swapDirection) {
            // probably there are other nodes on this line
            lastCorrentNode = nextNode
            continue
        }
        // have gone too far otherwise
        break
    }
    return lastCorrentNode
}

@AgentRBY
Copy link
Contributor

The feature was more complicated than I thought, so the implementation is a bit delayed.

@zardoy
Copy link
Owner Author

zardoy commented Nov 30, 2022

Just made it possible to move whole statements for any language (including JSON), and it seems to be handling commas after moving very well: https://github.com/zardoy/vscode-move-statement, so I'm not priorizing this issue. On the other hand for existing workflows, when user uses existing builtin move lines commands we can also help him to manage commas. So I'm up for implementing this one just to make out-of-the-box experience even better.

@AgentRBY
Copy link
Contributor

This extension (like your solution above) works well on single-level JSON. But when the transition from one level to another appears problems. That's why I have a delay in implementing this feature (I also have a deadline at work now, so I can't devote much time to extensions)

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

No branches or pull requests

2 participants