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

Fix: indent guides out of bound. #3105

Closed
wants to merge 2 commits into from

Conversation

erasin
Copy link
Contributor

@erasin erasin commented Jul 19, 2022

when Insufficient width of split window
WX20220719-145548@2x

when Insufficient width of split window
&indent_guide_char,
indent_style,
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I would think.. once it goes out_of_bounds the next indent_level can not suddenly be inside again.. or ??

Suggested change
}
} else {
break;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks , i test this, If use break, the indents of the next level will not be rendered.

Comment on lines +426 to +432
if i < hide_indent_level {
continue;
}

let visual_x = i * tab_width as u16;
let out_of_bounds =
visual_x < offset.col as u16 || visual_x >= viewport.width + offset.col as u16;
Copy link
Member

Choose a reason for hiding this comment

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

This seems overly complicated to me, you should be able to calculate the starting indent level outside the for loop then for i in starting_level..(indent_level / tab_width as u16) to start rendering at the right offset.

Copy link
Contributor

@aikomastboom aikomastboom Jul 20, 2022

Choose a reason for hiding this comment

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

It's not about the starting level.. it's about stopping and not exceeding as I understand.
Possibly, once the right limit has been found, code can be further optimized to reuse this value.
As the guides are an optional feature, optimization should not bleed into normal codeflow.

Copy link
Member

Choose a reason for hiding this comment

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

It's about the starting level. The code is rendering the right split's indent guides, but since the starting column is further in the file it's rendering them off screen into the previous split.

archseer added a commit that referenced this pull request Jul 20, 2022
@archseer
Copy link
Member

Addressed in 906259c

@archseer archseer closed this Jul 20, 2022
@erasin erasin deleted the indent-guides-out-of-bound branch July 20, 2022 11:07
aikomastboom pushed a commit to aikomastboom/helix that referenced this pull request Jul 20, 2022
@erasin erasin restored the indent-guides-out-of-bound branch July 22, 2022 11:41
@erasin erasin deleted the indent-guides-out-of-bound branch July 22, 2022 11:42
thomasskk pushed a commit to thomasskk/helix that referenced this pull request Sep 9, 2022
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.

3 participants