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

Fast current_indent implementation #703

Closed

Conversation

Danielkonge
Copy link
Contributor

This is the most up to date implementation mentioned in #649, and I don't feel any slowdown compared to the version without current_indent.

Please let me know if you want me to change something, then I can edit it during the next few days.

@Danielkonge
Copy link
Contributor Author

Even if you end up not accepting this pull request, I think it is worth considering changing utils similar to what I have done with start_pos_scope and end_pos_scope in utils.lua. It worked more consistently for me at least. (Even without the old definitions too, I just left them to have more chances of hitting the color you want.)

@lvim-tech
Copy link

lvim-tech commented Oct 8, 2023

@Danielkonge thanks for your work. I think indentation doesn't work correctly in all cases

@Danielkonge
Copy link
Contributor Author

Danielkonge commented Oct 8, 2023

I found a bug where current_indent wasn't correct if you jumped back more than one indent level in a single line. I have fixed it now. I think that is what affected you too (going two levels back from line 370 to line 371). Could you let me know if the problem is fixed in the newest commit?

@lvim-tech
Copy link

@Danielkonge now work correctly!

@Danielkonge
Copy link
Contributor Author

By now this commit has actually got a few different features mixed into it.

It has current_indent as stated in the title of this pull request, but since this is really just my working implementation of indent-blankline, more stuff has gotten included.

It now also adds a show_exact_scope and a current_block_only option + some improvements to the highlight hook for the scope.

Would you prefer that I split these out into multiple smaller pull requests @lukas-reineke? Or is it easier for you to do it all at once in one bigger file (like now)?

@lukas-reineke
Copy link
Owner

lukas-reineke commented Oct 10, 2023

Please make smaller PRs 👍
And it would be nice if you can write some tests for it too

@Danielkonge
Copy link
Contributor Author

@lukas-reineke I have submitted 3 smaller pull requests now, and I will add another smaller one with only current_indent later (I will leave this one open until that one is ready).

I am not sure how exactly you want to test these features, but if you clarify what kind of tests you want, I can try to write some tests too. [I didn't see any tests for scope to base my tests on.]

@Danielkonge
Copy link
Contributor Author

I finished the new smaller and cleaner pull requests now.

@Danielkonge Danielkonge deleted the current_indent_fork branch December 15, 2023 11:51
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