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

Add :retab command #1846

Merged
merged 1 commit into from
Dec 18, 2023
Merged

Add :retab command #1846

merged 1 commit into from
Dec 18, 2023

Conversation

FrostyX
Copy link
Contributor

@FrostyX FrostyX commented Dec 9, 2023

See "Changing tabs" section in the Vim manual
https://vimhelp.org/change.txt.html#change.txt

@tomdl89
Copy link
Member

tomdl89 commented Dec 12, 2023

Thanks fort the PR @FrostyX . Please add some tests so a) I can easily see what your implementation does and b) to guard against regressions in the future. Cheers

@FrostyX
Copy link
Contributor Author

FrostyX commented Dec 17, 2023

Thank you very much for the review @tomdl89,
I was a bit worried that it will be complicated to write the tests but I was sooo wrong. The tests are so comfortable to write thanks to the evil-test-buffer.

Updated, PTAL :-)

@tomdl89
Copy link
Member

tomdl89 commented Dec 17, 2023

Looks great - thanks for the tests @FrostyX ! Just need to get that Retab only selected lines test passing and I'm happy to merge :)

@FrostyX
Copy link
Contributor Author

FrostyX commented Dec 17, 2023

Yeah, I am trying to figure out what is wrong. As these things usually go, the test works fine on my machine.

@tomdl89
Copy link
Member

tomdl89 commented Dec 17, 2023

FWIW, they pass on my machine too with make emacs and make terminal but not with make test (which is what github actions uses), so the --batch option may be causing some differences.

@tomdl89
Copy link
Member

tomdl89 commented Dec 17, 2023

OK, having looked at this, and got it passing for each, but never together, I'd be happy to remove this test. It shows a difference between --batch mode and normal mode which I don't understand. A minimal repro (without evil) for the emacs dev mailing list may be interesting if you can't work out what causes the difference. Up to you.

@FrostyX
Copy link
Contributor Author

FrostyX commented Dec 17, 2023

Thank you very much for trying to debug the issue.

I'd be happy to remove this test.

Do you mind if we skip the tests in the batch mode instead? I updated the PR. I think it is worth showing what the command is supposed to do, as you said in

a) I can easily see what your implementation does

See "Changing tabs" section in the Vim manual
https://vimhelp.org/change.txt.html#change.txt
@tomdl89 tomdl89 merged commit fc6bd00 into emacs-evil:master Dec 18, 2023
4 checks passed
@tomdl89
Copy link
Member

tomdl89 commented Dec 18, 2023

Yup, all good. Thanks again for the PR!

@FrostyX
Copy link
Contributor Author

FrostyX commented Dec 18, 2023

Thank you very much.

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