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

Skip inline diff wording when > 10 000 hunk #1134

Closed
wants to merge 2 commits into from
Closed

Conversation

stoivo
Copy link
Member

@stoivo stoivo commented Jun 19, 2019

fix: #1133

@kaste, This is't perfect but it helps. The SequenceMatcher.get_opcodes call is the slow one in this case When I run it on a file with 30000 charachets with one characher changes it takes 35 sec om my mac book pro. We could "solve" by backgrounding. I dont think is it worst the CPU time even.

If you want to source controll a 30000 character long line manually please split it up. I also this SequenceMatcher uses more time it the are move changes between the lines.

An onther solution would be to use gits --word-diff. SequenceMatcher does a better job at finding the excat characher which changes. Git word-diff is a lot faster but less exact.

ok, fix?

@kaste
Copy link
Collaborator

kaste commented Jun 19, 2019

Bail out is okay, it still should run on the worker (even if this process only takes 1 second).

@asfaltboy
Copy link
Member

For what it's worth, I get the same issue when committing an ipynb file and have verbose commits turned on...
This PR seems to fix only issue with the diff in ipynb files by bailing out (though I haven't tried 100MB file), but it doesn't touch verbose diff rendering, though, it might be out of scope, and we ought to recommend users to mark such files as binaries in gitattributes

@stoivo
Copy link
Member Author

stoivo commented Jun 29, 2019

@kaste I agree.

@asfaltboy is it show_full_commit_info we are talking about. I also have show_full_commit_info enabled. Maybe we should do something similar to what @kaste did on the status view. The async rendering.

@stoivo stoivo force-pushed the inline_diff_big_files branch 2 times, most recently from 5a3a6f7 to 42a5b68 Compare June 29, 2019 22:48
@stoivo
Copy link
Member Author

stoivo commented Jun 29, 2019

@kaste I did one attemt but It feels like it is flacking which is annoying. Do you have a hint to me?

@kaste
Copy link
Collaborator

kaste commented Jul 1, 2019

What exactly do you mean with 'flacking'? On first usage, it should open a new blank view, then do the work in the background, and paint once when it's ready.

@stoivo
Copy link
Member Author

stoivo commented Jul 2, 2019

Sorry, flashing. I mean flashing.

Take a look at the screen recoring.
Archive 2.zip
If you look at it frame bye frame there are 2 frames where the text is selectected. On my case it would be pink. Somewhat bright and flashing.

@kaste
Copy link
Collaborator

kaste commented Jul 2, 2019

Do you mean such a frame:

image

I see the flashing, interesting enough in the above picture only the minimap is purplish, the actual buffer looks fine.

@kaste
Copy link
Collaborator

kaste commented Jul 2, 2019

Hm, there is no cursor. You should do

         self.view.run_command("gs_replace_view_text", {
-            "text": inline_diff_contents
+            "text": inline_diff_contents,
+            "restore_cursors": True
         })

@stoivo
Copy link
Member Author

stoivo commented Jul 4, 2019

This is what I mean, there are like 2-3 frames with each
Screen Shot 2019-07-03 at 08 03 59
Screen Shot 2019-07-03 at 08 04 22
Screen Shot 2019-07-03 at 08 04 33
Screen Shot 2019-07-03 at 08 04 43

@kaste
Copy link
Collaborator

kaste commented Jul 4, 2019

And the patch I posted ☝️ doesn't work?

@stoivo
Copy link
Member Author

stoivo commented Jul 5, 2019

no

@kaste
Copy link
Collaborator

kaste commented Jul 5, 2019

Hm, odd, I can reliable reproduce the selection issue you're showing, for the minimap if it is on. I get the whole selection only if the cursor is in block mode (since block mode is new, probably a bug).

But the patch "restore_cursors": True solves this on my Windows. (These are drawing bugs so every OS can have its own bugs.)

Please make sure to restart Sublime bc the reloader doesn't work reliable here.

@kaste
Copy link
Collaborator

kaste commented Jul 5, 2019

But keep in mind, it is not okay to always refresh async. Usually only on_activate and on_create. E.g. if we hunk something, we need to block the UI for that otherwise view and state get out of sync, and we probably cannot [hhh]unk anymore (just like what was the case for the diff view).

@kaste
Copy link
Collaborator

kaste commented Jul 5, 2019

There are other bugs as well, if you [h]unk just somewhere it will just throw at you. If you [h]unk just on the line before an actual hunk, it will still hunk. But if you [l]ine just before a changed line, it will actually stage the last line (iirc) from that hunk.

@stoivo
Copy link
Member Author

stoivo commented Jul 5, 2019

My bad, did'r restart sublime but only refreshed modules. My bad. Now it work perfectly. I added 2 commits to your branch. Lets contiune the review over there.

stoivo added a commit to kaste/GitSavvy that referenced this pull request Jul 5, 2019
@stoivo
Copy link
Member Author

stoivo commented Jul 5, 2019

Continutes in #1138

@stoivo stoivo closed this Jul 5, 2019
stoivo added a commit to kaste/GitSavvy that referenced this pull request Sep 24, 2019
@kaste kaste deleted the inline_diff_big_files branch November 25, 2020 13:20
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.

None yet

3 participants