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

Add support for copy/cut/paste #74

Closed
wojciechczerniak opened this issue Dec 10, 2019 · 6 comments
Closed

Add support for copy/cut/paste #74

wojciechczerniak opened this issue Dec 10, 2019 · 6 comments
Labels
Feature Something we can add later on without introducing a breaking change
Milestone

Comments

@wojciechczerniak
Copy link
Contributor

Description

Support for copy/cut/paste.

WIP. We need to specify how this feature should work.

@wojciechczerniak wojciechczerniak added Integration Feature Something we can add later on without introducing a breaking change labels Dec 10, 2019
@wojciechczerniak wojciechczerniak added this to the January 2020 milestone Dec 10, 2019
@voodoo11 voodoo11 mentioned this issue Jan 25, 2020
6 tasks
@voodoo11
Copy link
Collaborator

voodoo11 commented Jan 25, 2020

Current implementation.

public clipboardCopy(sourceLeftCorner: SimpleCellAddress, width: number, height: number): CellValue[][]
public clipboardCut(sourceLeftCorner: SimpleCellAddress, width: number, height: number): CellValue[][]
public clipboardPaste(targetLeftCorner: SimpleCellAddress): CellValueChange[]
public clipboardClear(): void

Copy:
As we are caching ASTs and relative dependencies while parsing formulas, we can store only formula r0c0 hashes to retriew proper objects when pasting. This simplifies and speeds up whole process. Current limitation is lack of distinction between relative and absolute sheet references. This mean that copying such formulas from Sheet1:

['=B1'],
['=Sheet1!B1']

to Sheet2, will result in

['=Sheet1!B1'],
['=Sheet1!B1']

Cut - paste:
Uses moveCells implementation. Almost any CRUD operation (except adding sheet) between cut and paste will clear internal clipboard and abort cut-paste.

Copy and cut returns cell values for further use in external clipboard.

@wojciechczerniak
Copy link
Contributor Author

wojciechczerniak commented Feb 6, 2020

@voodoo11, you were waiting for the names for methods. This should do it:

public copy(sourceLeftCorner: SimpleCellAddress, width: number, height: number): CellValue[][]
public cut(sourceLeftCorner: SimpleCellAddress, width: number, height: number): CellValue[][]
public paste(targetLeftCorner: SimpleCellAddress): CellValueChange[]
public clearClipboard(): void

IMO method this.clipboardOperations.abort() could be renamed to abortCut because it's what it does.

public abort(): void {
if (this.clipboard && this.clipboard.type === ClipboardOperationType.CUT) {
this.clear()
}
}

@wojciechczerniak
Copy link
Contributor Author

Copy:
As we are caching ASTs and relative dependencies while parsing formulas, we can store only formula r0c0 hashes to retriew proper objects when pasting. This simplifies and speeds up whole process. Current limitation is lack of distinction between relative and absolute sheet references. This mean that copying such formulas from Sheet1:

['=B1'],
['=Sheet1!B1']

to Sheet2, will result in

['=Sheet1!B1'],
['=Sheet1!B1']

Do we have any idea how we can fix this?

Cut - paste:
Uses moveCells implementation. Almost any CRUD operation (except adding sheet) between cut and paste will clear internal clipboard and abort cut-paste.

Right now we abort on every crud operation without knowing the result of such f.e:

public addRows(sheet: number, ...indexes: Index[]): void {
const normalizedIndexes = normalizeAddedIndexes(indexes)
this.ensureItIsPossibleToAddRows(sheet, ...normalizedIndexes)
this.clipboardOperations.abort()
for (const index of normalizedIndexes) {
this.doAddRows(sheet, index[0], index[1])
}
}

Maybe we could use our changes list?

private recomputeIfDependencyGraphNeedsIt(): ContentChanges {
const changes = this.crudOperations.getAndClearContentChanges()
const verticesToRecomputeFrom = Array.from(this.dependencyGraph.verticesToRecompute())
this.dependencyGraph.clearRecentlyChangedVertices()
if (verticesToRecomputeFrom.length > 0) {
changes.addAll(this.evaluator.partialRun(verticesToRecomputeFrom))
}
return changes
}

And compare the list against this.clipboard range? I think we've got everything we need to compare those two precisely, don't we?

class Clipboard {
constructor(
public readonly sourceLeftCorner: SimpleCellAddress,
public readonly width: number,
public readonly height: number,
public readonly type: ClipboardOperationType,
public readonly content?: ClipboardCell[][]
) {
}

Copy and cut returns cell values for further use in external clipboard.

Awesome idea 👍

@voodoo11
Copy link
Collaborator

Methods renamed #171

@voodoo11
Copy link
Collaborator

voodoo11 commented Feb 14, 2020

Current limitation is lack of distinction between relative and absolute sheet references. This mean that copying such formulas from Sheet1

Do we have any idea how we can fix this?

We should introduce sheet self-references.

Maybe we could use our changes list?
And compare the list against this.clipboard range? I think we've got everything we need to compare those two precisely, don't we?

It is not that simple, as changes list does not contain all the changes. For example if you add row (which can affect cut operation), all rows below are shifted, but this is only a structural change and nothing is added to the list. Otherwise we would have to add everything from this row to the end of the sheet.

@wojciechczerniak
Copy link
Contributor Author

Ok, I get it now. Thanks for the details

We should introduce sheet self-references.

I've created a feature request for self-references #179 if you could, please take a look.

I'm closing this task as done. Thanks 🏆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Something we can add later on without introducing a breaking change
Projects
None yet
Development

No branches or pull requests

2 participants