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

Issue loading additional files (after a file has already been loaded) #15

Open
gabrielle-ohlson opened this issue Mar 4, 2021 · 5 comments

Comments

@gabrielle-ohlson
Copy link
Contributor

gabrielle-ohlson commented Mar 4, 2021

Since pulling the most recent commit (as well as on the live visualizer website), I came across the following issue:
If I try to load a new file (file2) after having already loaded a different one (file1), the text-contents of file2 will load, but the visualization does not update to match the new file (instead shows visualization of file1 still).

Refreshing the page before loading a new file works as a quick fix, but this issue does not come up when I use an older version of the knitout-live-visualizer repo, so it seems like there's a bug in the newest commit.

@gabrielle-ohlson
Copy link
Contributor Author

By the way, here's the console error I get when I try to load a second file:

ace.js:8983 Uncaught TypeError: Cannot read property 'splice' of null
    at EditSession.Folding.updateFoldWidgets (ace.js:8983)
    at EditSession.EventEmitter._signal (ace.js:3454)
    at EditSession.onChange (ace.js:9338)
    at Document.EventEmitter._signal (ace.js:3454)
    at Document.applyDelta (ace.js:7379)
    at Document.$splitAndapplyLargeDelta (ace.js:7402)
    at Document.applyDelta (ace.js:7375)
    at Document.insertMergedLines (ace.js:7275)
    at Document.insert (ace.js:7203)
    at Document.setValue (ace.js:7109)
Folding.updateFoldWidgets @ ace.js:8983
EventEmitter._signal @ ace.js:3454
onChange @ ace.js:9338
EventEmitter._signal @ ace.js:3454
applyDelta @ ace.js:7379
$splitAndapplyLargeDelta @ ace.js:7402
applyDelta @ ace.js:7375
insertMergedLines @ ace.js:7275
insert @ ace.js:7203
setValue @ ace.js:7109
setValue @ ace.js:12765
reader.onload @ visualizer.html:456
load (async)
readFile @ visualizer.html:444
(anonymous) @ visualizer.html:556

@gabrielle-ohlson
Copy link
Contributor Author

@ixchow I just double-checked to see if this is an issue with the ace submodule and can confirm that it is not. (checked by replacing the ace-builds folder in a local clone of an earlier commit that doesn't have this issue with the ace folder from a clone of the newest commit. The second file still successfully loaded.)

Going to try to figure out what's going wrong with the git-bisect command, and also going to create a duplicate of the visualizer repo to try to fix this issue/make some error-checking improvements.

@gabrielle-ohlson
Copy link
Contributor Author

@ixchow I found the first bad commit: c916906dc624bad662a230b36c45422b6e3adce6. not sure what exactly in this commit caused the issue, but going to try to figure that out now.

@gabrielle-ohlson
Copy link
Contributor Author

update: the issue is with line 497:

editor.getSession().on('change', updateKnitoutMode);

as @ixchow pointed out, this line is meant to update the visualizer to change from js->knitout (or vice versa) when a user starts writing code in the editor that is a different language than the previous file.

when line 497 is simply removed, the visualizer no longer has issues loading a new file. at first, we thought that was the only situation in which this line of code becomes problematic, but I recently tried to type into the editor with knitout after a js file was loaded, and the same console error (ace.js:8983 Uncaught TypeError: Cannot read property 'splice' of null) came up.

so in summary, looks like its a problem with ace; potential solution: add a setTimeout() (TODO: try this out)

@gabrielle-ohlson
Copy link
Contributor Author

gabrielle-ohlson commented Mar 27, 2021

I think it is because ace has handling of onchange listeners that interferes with what we're trying to do with our own editor.getSession().on('change', updateKnitoutMode); listener. See the following code from ace.js:

    this.attach = function(session) {
        if (this.session)
            this.detach();

        this.session = session;
        this.onChange = this.$onChange.bind(this);

        this.session.on('change', this.onChange);
    };

    this.detach = function() {
        if (!this.session)
            return;
        this.session.removeListener('change', this.onChange);
        this.session = null;
    };

line 8014, this.session = null; might be what is causing the error, Cannot read property 'slice' of null

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

1 participant