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

Reworking line numbers + other fixes #2088

Closed
wants to merge 31 commits into from

Conversation

harshad1
Copy link
Collaborator

@gsantner @guang-lin

I reworked some of the line number stuff in this PR:

  • Simplified how the line number calculations and drawing are done.
  • Make the line number setting per-file

Please take a look and feel free to take anything you feel is useful.

guang-lin and others added 27 commits July 15, 2023 08:09
private ScrollView _scrollView;
private int _x;
private int _maxLineNumber = 1;
private int _maxLineNumberWidth;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed need for many vars

CharSequence deleted = s.subSequence(start, start + count);
matcher = pattern.matcher(deleted);
while (matcher.find()) {
_maxLineNumber--;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed need for change tracking

int number = 1;
int yPos = top;

while(line < maxLayoutLine && yPos < bottom) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplified loop. Basic logic is same though

@harshad1 harshad1 changed the title Do not merge: Reworking line numbers Reworking line numbers + other fixes Aug 13, 2023
@harshad1
Copy link
Collaborator Author

@guang-lin @gsantner This seems to be working fine except for line numbers in view mode. Line numbers in view mode appear to be broken in the original branch too though. Feel free to merge this branch if you prefer or pull changes from here into other branches if desired.

@guanglinn
Copy link
Contributor

It seems that you improved a lot, I will take a look carefully.

@gsantner
Copy link
Owner

gsantner commented Aug 14, 2023

would be also OK to merge the version fron guang-lin to master and then just harshads branch on top for improvements again

@harshad1
Copy link
Collaborator Author

That is perfectly fine by me. Note that this branch includes all of @guang-lin's commits

@harshad1
Copy link
Collaborator Author

Also note that I made some more changes to how we do the numbering this morning. It is somewhat more efficient / faster now. The earlier version it was possible to see some drops if you fast scrolled through a very large file

@gsantner gsantner changed the base branch from master to linenumbers August 14, 2023 22:44
@gsantner
Copy link
Owner

Changed the branch to linenumbers branch. Merged the initial PR and improvements can be continued via additional PRs to linenumbers branch. Then we can merge it to master when we think it's fine.

For this PR tried rebasing, but there were quite a lot conflicts. However a merge-resolve seems to be only resolving conflicts in 3 files.

@gsantner
Copy link
Owner

These changes are already integrated into guang's branch right? So this one can be closed I guess.

I think you also have the possibility to directly commit to the guang PR branch via git@github.com:guang remote.

@harshad1
Copy link
Collaborator Author

Closing as all commits have been moved

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