micro icon indicating copy to clipboard operation
micro copied to clipboard

Add wrapindent option to allow wrapping and hanging indents

Open protostork opened this issue 2 years ago • 9 comments

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.

protostork avatar Dec 30 '23 21:12 protostork

@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

Neko-Box-Coder avatar Jun 09 '24 00:06 Neko-Box-Coder

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.

protostork avatar Jun 09 '24 18:06 protostork

@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 avatar Jun 09 '24 18:06 Neko-Box-Coder

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.

Neko-Box-Coder avatar Jun 09 '24 19:06 Neko-Box-Coder

@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

protostork avatar Jun 14 '24 00:06 protostork

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

protostork avatar Jun 14 '24 02:06 protostork

  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.

protostork avatar Jun 14 '24 11:06 protostork

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.

Neko-Box-Coder avatar Jun 14 '24 13:06 Neko-Box-Coder