Skip to content

Conversation

@gjtorikian
Copy link
Contributor

Basically, avoid stack limit exceeding, and handle line updates in 100,000 chunks.

@javruben
Copy link

FYI: it is not so much performance as it is just ability to scale there...

@fjakobs
Copy link
Contributor

fjakobs commented May 10, 2012

Three times the same code. Can you put this in util/lang ?

@gjtorikian
Copy link
Contributor Author

@fjakobs Done.

Generalizing is hard. Each of the three are requiring and doing something different. This is the best solution (that I could come up with) that works.

@nightwing
Copy link
Member

maybe it would be better to include argument array creation into library function too
lang.spliceArray(startIndex, replaceCount, array)
lang.spliceArray(startIndex, replaceCount, length)

but then i am not sure what to do about https://github.com/ajaxorg/ace/blob/master/lib/ace/edit_session.js#L1649

@nightwing
Copy link
Member

another thought
if document emitted only short deltas only it will need to be modified
something like this, (can the reverse order be a problem here?)

    this.insertLines = function(row, lines) {
        if (lines.length == 0)
            return {row: row, column: 0};

        if (lines.length > 0xFFFF) {
            var end = this.insertLines(row, lines.slice(0xFFFF));
            lines = lines.slice(0, 0xFFFF);
        }

        var args = [row, 0];
        args.push.apply(args, lines);
        this.$lines.splice.apply(this.$lines, args);

        var range = new Range(row, 0, row + lines.length, 0);
        var delta = {
            action: "insertLines",
            range: range,
            lines: lines
        };
        this._emit("change", { data: delta });
        return end || range.end;
    };

btw https://bugs.webkit.org/show_bug.cgi?id=80797 says that safari accepts less than 100_000 is it so or 100000 is fine?

@gjtorikian
Copy link
Contributor Author

Closing this and merging into: #760

@gjtorikian gjtorikian closed this May 19, 2012
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

Successfully merging this pull request may close these issues.

4 participants