CleanCocoa / CleanCocoa/DeclarativeTextKit

Skip evaluation of a Modifying block iff it doesn't do anything

Ouverte
#9 5 commentaires 0 réactions 0 personnes assignées Voir sur GitHub
enhancement
Langage dominant
Swift
Étoiles
50
Forks
0
Métriques de merge des PR
Aucune PR mergée en 30 j

Description

The `Modifying() { }` construct always evaluates even if the block is empty.

With TextKit integration, this means

1. an undo group is being started (and ended)
2. `NSTextView.shouldChangeText(in:replacementString:)` and `didChangeText()` are being run to guard against unwanted changes

With syntax highlighting in the text storage, you may end up processing the text for what's essentially a no-op.

## How to test

To get an empty block, use a for-loop to trigger the `buildArray` path of the result builder, but without any actual iterations:

```swift
Modifying(selectedRange) { _ in
for _ in 0 ..< 0 {
Insert(0) { "loop never runs" }
}
}
```

I'm not sure whether we can figure out at all whether a result builder produces nothing (i.e. empty array).

## Complete test case

This test fails with a thrown error at `try buffer.evaluate` because the text view doesn't permit changes in the range.

This should not be a problem, because the range is not actually changed.

```swift
func testModifying_EmptyLoopBlock_SkipsEvaluation() throws {
class TextViewSpy: NSTextView {
var didCallShouldChangeText = false
var didCallDidChangeText = false

override func shouldChangeText(in affectedCharRange: NSRange, replacementString: String?) -> Bool {
didCallShouldChangeText = true
return false // Would abort modification an error
}

override func didChangeText() {
didCallDidChangeText = true
}
}

let textViewSpy = TextViewSpy()
textViewSpy.string = "Lorem ipsum."
let buffer = NSTextViewBuffer(textView: textViewSpy)
let selectedRange: SelectedRange = .init(location: 6, length: 5)

assertBufferState(buffer, "Lorem ipsum.ˇ")

try buffer.evaluate {
Modifying(selectedRange) { _ in
for _ in 0 ..< 0 {
Insert(0) { "loop never runs" }
}
}
}

XCTAssertFalse(textViewSpy.didCallShouldChangeText)
XCTAssertFalse(textViewSpy.didCallDidChangeText)
}
```

Guide de contribution

Aucun guide de contribution indexé pour ce dépôt

Piste de recherche

Commencez par le construct Modifying et le point d’entrée buffer.evaluate, puis examinez comment un bloc vide de result-builder est représenté pour le chemin buildArray. Reproduisez le problème avec testModifying_EmptyLoopBlock_SkipsEvaluation et le TextViewSpy fourni. C’est terminé lorsque le bloc de boucle vide ignore l’évaluation et qu’aucun des deux callbacks de changement de texte n’est appelé.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
swift
Domaine
desktop
Type d'issue
Bug
Difficulté
3/5
Temps estimé
1-2 jours
Activité
À l'abandon
Clarté
Plutôt claire
Accessibilité débutants
35/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.