Code viewer fails to show entire file #229

Closed
JamesDunne opened this Issue Sep 19, 2012 · 27 comments

Projects

None yet

5 participants

@JamesDunne

https://github.com/JamesDunne/rest0-api/blob/master/REST0.APIService/APIHttpAsyncHandler.cs

The viewer does it show the entire file. It does 5 fetches and stops around line 1,100.

@kevinsawicki

Hi, what version of the app are you running and also what phone and Android version are you on?

@JamesDunne

I'm using an unmodified Samsung Galaxy S3 with T-Mobile in the US, Android version 4.0.4. App version is 1.3, just updated within the last few days.

Can you reproduce this given the sample file link above?

@JamesDunne

Ah I see #222 now. Should've done a better job searching. I'm make sure my version gets up to 1.4 when my Google Play app picks it up and test it out then.

@kevinsawicki

@JamesDunne great thanks, feel free to leave this issue open until you confirm it does or does not work on 1.4.

@JamesDunne

Will do. I can't seem to get my Google Play to see your 1.4 version. It's still stuck on 1.3; is there some significant propagation delay or something? You claim to have uploaded it 2 days ago and I'm a bit surprised that it'd take this long.

@sagarsane

I just validated this on the latest Github App, pushed after #222 fix. It seems to pull out all 1842 lines of code :)

@kevinsawicki

It should show 1.4 here

1.3 is actually 2 releases behind now, there was a 1.3.1 as well.

@JamesDunne

I was going based on what the "What's New" section reads. It reads the last update was "1.3". I'm not sure how to check the "official" version number of the app installed on my phone though.

@JamesDunne

I just completely uninstalled and reinstalled the app, went to my sample file and it stops at line 1212.

@sagarsane

I believe this shows 1.4 on the page referenced by @kevinsawicki on the right hand side bar under About This App

@kevinsawicki

@JamesDunne , you should be able to see the app version from the Manage Apps menu on the Home Screen.

Select the GitHub app and it should display the version under the title.

@JamesDunne

Yes, it is reporting as 1.4 from the Application Manager under Settings. I have to conclude that it's still a bug then. :(

Is this bug specific to different language syntax highlighters? That would explain the unfriendliness to C# code :)

@joseds

This is strange, I thought the problem was fixed with version 1.4. But now it seems the magic number 101 (see #222 for its meaning) has changed to 202 (at least for me) and the behaviour is the same otherwise. (Samsung Galaxy Nexus, Android 4.1.1)

@kevinsawicki

So is this happening for all files? Only files with syntax highlighting? Or only files without syntax highlighting?

@JamesDunne

Honestly, I'm not sure. It's hard to find large non-highlighted files on GitHub :). But for large files with syntax highlighting it's definitely occurring and reproducible. Check neo/d3xp/Actor.cpp in TTimo/doom3.gpl repository.

I used to be able to repro it on a large non-highlighted config.hson file I had in an earlier commit in my repository but I've since rewritten that file to be shortened and the issue is not reproducible there anymore.

@joseds

In the public repository joseds/Test there is a file long_file.txt which contains a list from 1 to 10000, separated by CR/LF. On opening the file in the GitHub App, only the first 404 lines are visible. Switching between portrait and landscape modes shows additional 101 lines, zooming in and out shows 202 lines more each time.

@kevinsawicki

@JamesDunne and @joseds thanks for the details, will try again to reproduce and find another fix.

I did my initial testing on an HTC Thunderbolt running Gingerbread and the issue appeared to completely go away when adding the call to refresh() on the editor.

@sagarsane

@kevinsawicki .. I am finding this bug to be inconsistent. High probability that loading comolete file will fail if user scrolls too fast. It worls if user scrolls slowly. It worked for me if user scrolled slowly.
Would it be better if there is some indication to the user when next parts of a file are loading?

Sagar

@joseds

Thank you @kevinsawicki. I can offer to test your next fix on my old Motorola Droid (Honeycomb), too, I just need to raise it from the dead...

@JamesDunne

Just updated to version 1.5 and I'm able to scroll a little further down in my file now, down to line 1414. But it just stops at that point and won't load any more.

https://github.com/JamesDunne/rest0-api/blob/master/REST0.APIService/APIHttpAsyncHandler.cs

@JamesDunne

It's worth noting that the line number stopped at is always a multiple of 101.

@jolo72

I have looked into this some and it is definitely an issue with codemirror or the way it's being used. I am not familiar with codemirror at all so I hate to put blame there if it's not due. Large files take a long time to load on the github app because codemirror is highlighting them with a default of 200ms processing time, 300ms sleep time and only displaying up to 100 (new) lines. I have tried tweaking the 200ms and 300ms but never saw any improvement. I did notice that if I change the following line in updateDisplay:

      var from = Math.max(visible.from - 100, 0), to = Math.min(doc.size, visible.to + 100);

to be

      var from = Math.max(visible.from - 10000, 0), to = Math.min(doc.size, visible.to + 10000);

then the https://github.com/JamesDunne/rest0-api/blob/master/REST0.APIService/APIHttpAsyncHandler.cs file is fully loaded nearly right away. The downside is that it is not highlighted yet, but in my opinion, that's not a big deal as the highlighting does show up eventually. codemirror is doing a ton of work here and it just flat out takes a long time to do it's job.

Random thoughts:

  • I believe it to be those -100 and +100 hard coded values that cause the 'magic 101' issue.
  • I do not know why 100 was chosen to be the magic number.
  • I think the call to refresh that was added is not necessary.
@JamesDunne

Nice work, @jolo72! I've been seeing other apps that use codemirror have issues as well.

For the record - I've been meaning to break up this file into separate files... but for now it'll serve as a good test case for codemirror =P

@kevinsawicki

@jolo72 Thanks for all the details, would you mind pulling and building the latest master and see if you still see this issue?

I just pushed a commit to upgrade to CodeMirror 2.3.5 and I'm now no longer able to reproduce this issue on my Nexus 7.

@jolo72

I will try to give it a shot when I get some more time.

@jolo72

I have tried this again today with the latest code and I am also no longer able to reproduce the issue. I notice that I am able to scroll to the end of the file rather quickly and some time after that the contents of the file down there are highlighted, which means to me CodeMirror must have changed the code to show the whole file first and then highlight it, which is what I had mentioned could be done in my previous comment. Way nicer for it to be fixed by CodeMirror themselves.

@kevinsawicki

@jolo72 Thanks for letting me know, I will close this issue out and this will be fixed in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment