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

Support conflict-marker-size larger than 7 #25

Merged
merged 2 commits into from
May 19, 2024

Conversation

sybereal
Copy link
Contributor

conflict-marker-size is an attribute that allows overriding the length of conflict markers generated by Git. This PR adjusts the default regex patterns to support that scenario.

conflict-marker-size is a Git attribute that can be set on files to
adjust the length of the conflict markers. This change adjusts the
regular expressions to not match an exact length of 7, but rather a
minimum length of 7.
@rhysd
Copy link
Owner

rhysd commented May 19, 2024

Thanks. I didn't know this.

@rhysd rhysd merged commit e5ddd99 into rhysd:master May 19, 2024
rhysd added a commit that referenced this pull request May 19, 2024
…ersize"

because this change hits a bug of matchit.vim

This reverts commit e5ddd99, reversing
changes made to f3a26e8.
@sybereal
Copy link
Contributor Author

What is the issue with matchit that this triggers? I have admittedly never tried using the jump feature.
Additionally, I am using https://github.com/andymass/vim-matchup instead of native matchit, and I don't know if that is affected by the same bug.

@rhysd
Copy link
Owner

rhysd commented May 20, 2024

The issue is that matchit.vim doesn't handle escaped , correctly in patterns. This change introduced , in pattern (e.g. ^>>>>>>> to ^>\{7,}) so it hit the issue.
I'll make an issue on vim/vim repository and link it to this. Note that 0e9686f is a temporary workaround for the issue. Once the issue is solved, I'll revert the revert commit again.

@sybereal
Copy link
Contributor Author

Ah, I see. I can rewrite the expressions without , using + instead later, if that helps.

@rhysd
Copy link
Owner

rhysd commented May 20, 2024

Oh, that sounds a good idea I could not come up with. I'll try it.

rhysd added a commit that referenced this pull request May 20, 2024
@rhysd
Copy link
Owner

rhysd commented May 20, 2024

bdc226c

I applied the idea at the above commit. It is already committed to the main branch.

@sybereal
Copy link
Contributor Author

Thank you for the swift response and resolution!

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

2 participants