singerdmx / flutter-quill

Rich text editor for Flutter
https://pub.dev/packages/flutter_quill
MIT License
2.6k stars 839 forks source link

Failed compose assertion on Document class #2099

Closed CatHood0 closed 3 months ago

CatHood0 commented 3 months ago

Is there an existing issue for this?

Short description

I'm trying to create a rule that is quite similar to AutoFormatMultipleLinksRule, however, it needs the whole line to be able to correctly match the regex pattern and allow me to apply any custom implementation on my part. However, there seems to be a problem when composing the Document with the newly generated Delta.

The problem

  'package:flutter_quill/src/document/document.dart': Failed assertion: line 418 pos 12: '_delta == _root.toDelta()': Compose failed
  dart:core                                                _AssertionError._throwNew
  package:flutter_quill/src/document/document.dart 418:12  Document.compose
  test/unit_test/script_mode/rules_test.dart 30:14         main.<fn>

When I try to insert my new line now formatted (this is something that already comes in another implementation of my project) with some block styles.

Initially, I use a Delta like this for testing:

Delta()
..insert('JOHN')
..insert('\n')
..insert('(angrily)')
..insert('\n')
..insert('this is a dialog')
..insert('\n')
..insert('INT. SCENE - DA')
..insert('\n');

The Rule should (when inserting the following data) format the insert with the text: "INT. SCENE - DA" with some attributes in block and add the last text.

Such that:

Delta()
..insert('JOHN')
..insert('\n')
..insert('(angrily)')
..insert('\n')
..insert('this is a dialog')
..insert('\n')
..insert('INT. SCENE - DA')
..insert('\n', {"align":"center","indent": 2})
..insert('\n');

However, no matter what I do, I am not able to solve an error when composing the Document.

@AtlasAutocode since you know how Rules work much better than me, how could I solve this, or do you have any suggestions? This would help me a lot as it would also allow me to understand more about this part and add more documentation to the project.

This is the Test that i using for:

void main() {
  late Delta delta;

  setUp(() {
    delta = Delta()
      ..insert('JOHN')
      ..insert('\n')
      ..insert('(angrily)')
      ..insert('\n')
      ..insert('this is a dialog')
      ..insert('\n')
      ..insert('INT. SCENE - DA')
      ..insert('\n');
  });

  test('should', () async {
    final rules = ScriptRules(onUpdatePosition: () {
      return (ScriptPosition.no_position, ScriptSettings.common());
    });
    final document = Document.fromDelta(delta);
    final newDelta = rules.apply(RuleType.insert, document, 47, len: 1, data: 'y');
    document.compose(newDelta, ChangeSource.local);

    print(document.toDelta());
  });
}

In any case, here is the rule I am using:

Rule class:

class AutomaticFormatSceneHeading extends BaseScriptRule {
  AutomaticFormatSceneHeading({
    required super.position,
    required super.onUpdatePositionInRule,
    required super.settings,
  });

  /// Scene heading pattern.
  ///
  /// This pattern is used to match a scene heading within a text segment.
  ///
  /// It works for the following testing URLs:
  ///
  /// INT. OFICINA - NOCHE
  /// EXT. PARQUE - DÍA
  /// INT./EXT. COCHE EN MOVIMIENTO - DÍA
  /// EXT. CASA > INT. CASA - DÍA
  /// INSERT: PANTALLA DEL TELÉFONO MÓVIL
  /// MONTAGE: ENTRENAMIENTO DE BOXEO DE JOHN

  static const _sceneHeadingPattern =
      r'\b(?:(INT|EXT|INT\/EXT|INSERT|MONTAGE|POV|SERIES OF SHOTS|INTERCUT|ESTABLISHING)(\.))\s+([A-Z0-9 \/\-]+?)\s+(-)?\s+([A-Z]*)\b';

  /// It requires a valid link in one scene heading
  RegExp get detectSceneHeadingPattern => RegExp(
        _sceneHeadingPattern,
        caseSensitive: false,
      );

  @override
  Delta? applyRule(
    Document document,
    int index, {
    int? len,
    Object? data,
    Attribute? attribute,
    Object? extraData,
  }) {
    // Only format when inserting text.
    if (data is! String) return null;
    String text = data;
    final sceneScriptElement = settings.firstWhere((element) => element.id == 'Scene-heading-0');
    if (sceneScriptElement.font.allCaps) {
      text = text.toUpperCase();
    }

    // Get current text.
    final entireText = document.toPlainText();

    // Get word before insertion.
    final leftWordPart = entireText
        // Keep all text before insertion.
        .substring(0, index)
        // Keep last paragraph.
        .split('\n')
        .last
        .trimLeft();

    // Get word after insertion.
    final rightWordPart = entireText
        // Keep all text after insertion.
        .substring(index)
        // Keep first paragraph.
        .split('\n')
        .first
        .trimRight();
    // Build the segment of affected words.
    final affectedWords = '$leftWordPart$text$rightWordPart';

    var usedRegExp = detectSceneHeadingPattern;
    // Check for scene heading pattern.
    final matches = usedRegExp.allMatches(affectedWords);

    // If there are no matches, do not apply any format.
    if (matches.isEmpty) return null;

    // Get unchanged text length.
    final unmodifiedLength = index - leftWordPart.length;
    // The base delta is a simple insertion delta.
    final baseDelta = Delta()
      ..retain(unmodifiedLength)
      ..insert('\n');

    // This retain just maintain that part of the text without changes
    final formatterDelta = Delta()..retain(unmodifiedLength + 1);

    for (final match in matches) {
      // Get the matched scene heading.
      final sceneHeading = affectedWords.substring(match.start, match.end);

      formatterDelta.insert(
          sceneHeading,
          {
            'bold': sceneScriptElement.font.bold,
            'italic': sceneScriptElement.font.italic,
            'underline': sceneScriptElement.font.underline,
          }.ignoreIf(predicate: (element, value) => value == false || value == null));

      formatterDelta.insert('\n', {
        'align': sceneScriptElement.paragraph.alignment,
        'line-height': _getLineHeight(sceneScriptElement.paragraph.lineSpacing),
        'indent': sceneScriptElement.paragraph.spaceBefore == 0 ? null : sceneScriptElement.paragraph.spaceBefore,
      });
    }

    // Build and return resulting change delta.
    return baseDelta.compose(formatterDelta);
  }

  @override
  RuleType get type => RuleType.insert;

  @override
  void validateArgs(int? len, Object? data, Attribute? attribute) {
    assert(data != null);
  }

  double _getLineHeight(String lineHeight) {
    return lineHeight.equals('single')
        ? 1.0
        : lineHeight.equals('multiple') || lineHeight.equals('double')
            ? 2.0
            : double.parse(
                lineHeight,
              );
  }
}

BaseScriptRule class:

abstract class BaseScriptRule extends Rule {
  final ScriptPosition position;
  final void Function(ScriptPosition) onUpdatePositionInRule;
  final ScriptSettings settings;

  BaseScriptRule({
    required this.position,
    required this.onUpdatePositionInRule,
    required this.settings,
  });
}
AtlasAutocode commented 3 months ago

Rules are a mysterious black box that do things that are useful but work in mysterious ways (I am trying to be polite here as I usually end-up scratching my head to figure out what they are doing!). Also, rules may interact in unexpected ways, so you may want to try to isolate your rule by disabling all the other rules - or calling your rule first and not return null (null return allows the other rules to execute).

..insert('\n', {"align":"center","indent": 2})

This might be causing a problem. Indenting does not appear to be working correctly. Indenting is correct for left-align. Indenting has no effect on right aligned text (BUG?). Indenting center alignment appears to shift in a way that is not obvious and might be buggy. Can you avoid the indenting and see if it solves the problem?

document compose has a big warning

  /// Composes [change] Delta into this document.
  ///
  /// Use this method with caution as it does not apply heuristic rules to the
  /// [change].
  ///
  /// It is callers responsibility to ensure that the [change] conforms to
  /// the document model semantics and can be composed with the current state
  /// of this document.
  ///
  /// In case the [change] is invalid, behavior of this method is unspecified.

The assert at line 418 indicates compose failed, presumably you are trying to do something that the document model semantics does not like.

In these situations, I usually take the example app with the empty option and use hard coding to pre-stuff the empty document with the deltas that I want to end up with. I can then see how it is rendered. This would confirm that the deltas are valid.

You could also place a breakpoint on Line 414 where compose is called and then step into the code to see why the compose fails. What is weird is that the catch is not triggered so the call to compose is returning something other than the root deltas.

I will try to take another look at the details of your rule, but I wanted to give you some feedback quickly.

CatHood0 commented 3 months ago

This might be causing a problem. Indenting does not appear to be working correctly. Indenting is correct for left-align. Indenting has no effect on right aligned text (BUG?). Indenting center alignment appears to shift in a way that is not obvious and might be buggy. Can you avoid the indenting and see if it solves the problem?

I will take a look about it. And yeah, i saw that indent works weird when alignment attribute is applied to the line.

In these situations, I usually take the example app with the empty option and use hard coding to pre-stuff the empty document with the deltas that I want to end up with. I can then see how it is rendered. This would confirm that the deltas are valid.

Thanks, that was useful. I'll try your suggestion.

CatHood0 commented 3 months ago

@AtlasAutocode i already notice what is the issue.

This part of the code has a relevant issue: a nullable value.

  formatterDelta.insert('\n', {
        'align': sceneScriptElement.paragraph.alignment,
        'line-height': _getLineHeight(sceneScriptElement.paragraph.lineSpacing),
        'indent': sceneScriptElement.paragraph.spaceBefore == 0 ? null : sceneScriptElement.paragraph.spaceBefore,
      });

If you pass a null value to any key into the attributes map, compose operation will failure. But, if you put a non null value, or directly remove null values from the map, compose operation will end successfully. Thanks for take care about my issue.

AtlasAutocode commented 3 months ago

Sounds good and makes sense. null value attributes seem to be used internally to flag that an attribute should be removed. Which suggests that quill does not want null value attributes in the actual deltas.

I looked at the indenting issue. On Microsoft Word, center aligned indenting appears to be half the spacing of left alignment. Right aligned text does NOT indent. So, I am calling flutter-quill behavior correct and not a bug (even though is does seem weird). I am not going to touch RTL!