Skip to content

Conversation

@gcanat
Copy link
Contributor

@gcanat gcanat commented May 19, 2025

I was running into some highlighting issues as shown in this demo.
I was able to fix it by replacing foldmethod=syntax with foldmethod=expr which allowed me to remove the syn region definitions in the syntax file.
The fold function is adapted from vim-diff-fold

I took the liberty to make some syn match a bit more restrictive and also changed the highlight groups for prrAdded and prrRemoved to the standard diff groups which feels more natural when going through diffs.

Happy to discuss any modifications or improvement.
And thanks for this cool tool 😃

@danobi
Copy link
Owner

danobi commented May 21, 2025

Acknowledging I see your PR! Will take a look tomorrow

Copy link
Owner

@danobi danobi left a comment

Choose a reason for hiding this comment

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

I'm no vimscript expert (or even beginner), but I tested this and it looks correct. Nice to be able to simplify the region specific logic.

Just one nit.


let b:undo_ftplugin = 'setl fdm&'
" Adapted from https://github.com/sgeb/vim-diff-fold
function! DiffFoldLevel()
Copy link
Owner

Choose a reason for hiding this comment

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

Should be able to make this script local, right?

Suggested change
function! DiffFoldLevel()
function! s:DiffFoldLevel()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done!

Copy link
Owner

@danobi danobi left a comment

Choose a reason for hiding this comment

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

Thank you!

@danobi danobi merged commit 9636d5f into danobi:master May 22, 2025
3 checks passed
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.

2 participants