Skip to content

Conversation

@protostork
Copy link

Possible implementation of feature request described in #2935 to allow configurable wrapping indents / hanging indents. Hope it corresponds with the project's coding styles and contribution guidelines.

Pull request adds the option wrapindent, which is -1 by default, where there is no change from the current default behaviour of no wrapping indents. Once wrapindent is set to 0 or greater, the new wrapped line respectively inherits or adds to the indent level of the parent line.

This seems to work fine for wordwrap: true / false, and softwrap: true. However, with wordwrap: false, it might occasionally add an additional empty space, if the line wrap is on a whitespace character, which might only be a minor quibble.

@Neko-Box-Coder
Copy link
Contributor

Neko-Box-Coder commented Jun 9, 2024

@metastork
Could you update your branch to fix the conflicts? I can give this a test after that and then hopefully can get this merged

@protostork
Copy link
Author

Hi @Neko-Box-Coder, thanks for taking a look. Have updated the branch from the upstream master, hope this works ok.

Having used this feature for the past few months, everything seems to work fine, except for a minor niggle - if using wrapindent = 0 or 4, when moving up and down with arrow keys, the cursor will jump slightly diagonally, rather than straight down.

It hasn't really bothered me but I'd be happy to solve this, either in this branch or in a future pull request, if you think it's worth doing.

@Neko-Box-Coder
Copy link
Contributor

Neko-Box-Coder commented Jun 9, 2024

@metastork

Thanks, I have quickly looked at the changes and it seems good. It would be great if you could sort out the cursor traversal when wrapindent is 0 or 4. You probably just need to add the indent amount when moving the cursor up or down in cursor.go.

I can do a detailed review and test when you fix the issue. If you can't, I can give it a go as well.

@Neko-Box-Coder
Copy link
Contributor

You probably just need to add the indent amount when moving the cursor up or down in cursor.go.

Probably to do with VisualX I think.

@protostork
Copy link
Author

@Neko-Box-Coder Thank you for the suggestions, that was helpful! (and trickier than expected :)

Have made another commit that should fix the diagonal cursor jumping when wrapindent > -1. Some notes:

  • cursor.go unfortunately doesn't get invoked when softwrap is on, so have integrated directly into the cursor up and down actions in actions.go (with some help in softwrap.go to deal with wordwrap on).
  • have put an AdjustXCursorOnWrapindent helper function into bufpane.go, which seemed the least-bad place for it, but of course happy to consider suggestions if you have better ideas

Copy link
Contributor

@Neko-Box-Coder Neko-Box-Coder left a comment

Choose a reason for hiding this comment

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

Currently, there are 2 behaviors that are wrong from my testing

  1. Going up between paragraphs does not retain the cursor Visual X position as pointed out in the code

  2. If wrapindent is >0, if you go down from the beginning in the first row to the second row, the cursor will just goes to the end of the first row instead of going to the beginning of the second row.

For example, let's say we have wrapindent of 4 and the capital letter C is our cursor, and small letters x are the text

Cxxxxxxxxxxxxxxx
    xxxxxxxxxxxx
    xxxxxxxxxxxx

If my cursor goes down, I would expect it to be

xxxxxxxxxxxxxxxx
    Cxxxxxxxxxxx
    xxxxxxxxxxxx

But instead, it is

xxxxxxxxxxxxxCxx
    xxxxxxxxxxxx
    xxxxxxxxxxxx

Maybe there needs to be a check or change in logic for the AdjustXCursorOnWrapindent.

@protostork
Copy link
Author

Thank you, you're right, good suggestions, will take a look and submit some fixes for those and some other edge cases.

@protostork
Copy link
Author

  1. Going up between paragraphs does not retain the cursor Visual X position as pointed out in the code
  2. If wrapindent is >0, if you go down from the beginning in the first row to the second row, the cursor will just goes to the end of the first row instead of going to the beginning of the second row.

Thanks for your help and detailed testing, there was indeed a bug in the implementation that had some knock-on effects such as these (getLocFromVLoc also needed to check for leading white space, not just getVLocFromLoc.

In the latest commit this should now be working properly in all scenarios. Please do have a look some time and let me know any suggestions.

@Neko-Box-Coder
Copy link
Contributor

Neko-Box-Coder commented Jun 14, 2024

Nice, the cursor up and down is now working between paragraphs (1.)
The second point is still not working

This is my test case:

Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line
Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line
    Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line

Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line
Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line Just a Very Long Line

Other than that, it seems working to me.

Copy link
Contributor

@Neko-Box-Coder Neko-Box-Coder left a comment

Choose a reason for hiding this comment

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

👍

Just need either @dmaluka or @JoeKar to approve and merge this if this looks good

@dmaluka
Copy link
Collaborator

dmaluka commented Sep 13, 2024

This is what I'm getting with wrapindent >= 0 and wordwrap enabled:

image

i.e. if the first word in a visual line doesn't fit in the window, this word is not wrapped, and even overwrites the adjacent pane.

Also, with hltaberrors enabled, this is what I'm getting when I shrink the pane so much that even the leading whitespace doesn't fit:

image

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