Skip to content

ArgList: Added argdedupe command to remove duplicate arguments#6235

Closed
nir9 wants to merge 8 commits intovim:masterfrom
nir9:master
Closed

ArgList: Added argdedupe command to remove duplicate arguments#6235
nir9 wants to merge 8 commits intovim:masterfrom
nir9:master

Conversation

@nir9
Copy link
Contributor

@nir9 nir9 commented Jun 11, 2020

Fixing the issue #6210
Added ":argdedupe" command that cleans the args from any duplicates.

@nir9 nir9 closed this Jun 11, 2020
@nir9 nir9 reopened this Jun 13, 2020
@nir9 nir9 changed the title ArgList: Remove duplicate arguments ArgList: Dont add duplicate arguments when doing argadd or argedit Jun 13, 2020
@nir9
Copy link
Contributor Author

nir9 commented Jun 13, 2020

This PR adds the ability to ignore duplicate files added with argadd and argedit, if a user attempts to argedit a file that already exists in the args it will not be added to the args and the user will move to the already existing entry. I fixed the tests accordingly.

@brammool
Copy link
Contributor

Problem is that this is not backwards compatible. Some users (and plugins) may depend on the current behavior.
I'm not sure what would be the best way forward:

  • Add a ++dedupe option
  • Keep the commands as-is and add an :argdedupe command to clean it afterwards
  • Add new commands that have the dedupe behavior

I tend to think that adding :argdedupe might be the best, so you can do ":argedit file | argdedupe"
Or just type :argdedupe once you find out it's desired.

@nir9 nir9 changed the title ArgList: Dont add duplicate arguments when doing argadd or argedit ArgList: Added argdedupe command to remove duplicate arguments Jun 15, 2020
@nir9
Copy link
Contributor Author

nir9 commented Jun 15, 2020

@brammool Added the argdedupe according to your suggestion, also added a test for the new command

@nir9 nir9 closed this Dec 21, 2021
@brammool
Copy link
Contributor

Did you close it just because it was old? There was a note in the todo list, but it has gone down a bit because new things got added.

@nir9
Copy link
Contributor Author

nir9 commented Dec 21, 2021

Oh this is actually by mistake I forgot about this and rebased my forks master branch (I am now working on a different fix) dropping these commits XD so I guess the GitHub automatically closed this pull request

I will soon try to restore the commits (looks like they still exist on GitHub) and put them on a different branch and reopen this pull request so they don't get lost.

@nir9 nir9 reopened this Dec 21, 2021
@nir9
Copy link
Contributor Author

nir9 commented Dec 21, 2021

Hi, I have re-added the original commits, though I have removed the changes I did to the tags and cmdidxs since you can anyway just compile them again easily so no need for them to be included in the changes (they can be compiled once you decide to add this to vim)

@brammool
Copy link
Contributor

Please do include the changes to cmdidxs, so that CI can run and test the change.

@nir9
Copy link
Contributor Author

nir9 commented Dec 23, 2021

@brammool Added the generated cmdidxs :)

@codecov
Copy link

codecov bot commented Dec 23, 2021

Codecov Report

Merging #6235 (13b8cd5) into master (1b5f7a6) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6235      +/-   ##
==========================================
- Coverage   90.38%   90.38%   -0.01%     
==========================================
  Files         154      154              
  Lines      173065   173173     +108     
==========================================
+ Hits       156421   156518      +97     
- Misses      16644    16655      +11     
Flag Coverage Δ
huge-clang-none 89.38% <100.00%> (-0.01%) ⬇️
huge-gcc-none 89.82% <100.00%> (-0.01%) ⬇️
huge-gcc-testgui 88.40% <100.00%> (+<0.01%) ⬆️
huge-gcc-unittests 2.45% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/arglist.c 92.38% <100.00%> (+0.15%) ⬆️
src/sound.c 96.15% <0.00%> (-0.97%) ⬇️
src/strings.c 96.28% <0.00%> (-0.34%) ⬇️
src/vim9compile.c 95.17% <0.00%> (-0.24%) ⬇️
src/vim9instr.c 90.22% <0.00%> (-0.21%) ⬇️
src/sign.c 94.61% <0.00%> (-0.20%) ⬇️
src/os_unix.c 72.37% <0.00%> (-0.09%) ⬇️
src/viminfo.c 93.51% <0.00%> (-0.07%) ⬇️
src/ex_cmds.c 93.01% <0.00%> (-0.05%) ⬇️
src/terminal.c 91.52% <0.00%> (-0.04%) ⬇️
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b5f7a6...13b8cd5. Read the comment docs.

@brammool brammool closed this in 73a0242 Dec 24, 2021
zeertzjq added a commit to zeertzjq/neovim that referenced this pull request Aug 16, 2022
Problem:    The argument list may contain duplicates.
Solution:   Add the :argdedeupe command. (Nir Lichtman, closes vim/vim#6235)
vim/vim@73a0242

Use latest index.txt :argdedeupe doc from Vim.
zeertzjq added a commit to zeertzjq/neovim that referenced this pull request Aug 16, 2022
Problem:    The argument list may contain duplicates.
Solution:   Add the :argdedeupe command. (Nir Lichtman, closes vim/vim#6235)
vim/vim@73a0242

Use latest index.txt :argdedupe doc from Vim.
zeertzjq added a commit to neovim/neovim that referenced this pull request Aug 16, 2022
Problem:    The argument list may contain duplicates.
Solution:   Add the :argdedeupe command. (Nir Lichtman, closes vim/vim#6235)
vim/vim@73a0242

Use latest index.txt :argdedupe doc from Vim.
smjonas pushed a commit to smjonas/neovim that referenced this pull request Dec 31, 2022
…19795)

Problem:    The argument list may contain duplicates.
Solution:   Add the :argdedeupe command. (Nir Lichtman, closes vim/vim#6235)
vim/vim@73a0242

Use latest index.txt :argdedupe doc from Vim.
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