[vim/vim] Add `:uniq` Ex-command (PR #17538)
h_east
To all English speakers>
please review the document. Words, phrases, etc.
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/17538
Commit Summary
- 1896e91 Add `:uniq` Ex-command
File Changes
(10 files)
- M runtime/doc/builtin.txt (1)
- M runtime/doc/change.txt (26)
- M runtime/doc/index.txt (1)
- M runtime/doc/tags (7)
- M src/ex_cmdidxs.h (14)
- M src/ex_cmds.c (118)
- M src/ex_cmds.h (3)
- M src/proto/ex_cmds.pro (1)
- M src/testdir/Make_all.mak (2)
- A src/testdir/test_uniq.vim (329)
Patch Links:
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Yegappan Lakshmanan
@yegappan requested changes on this pull request.
In src/ex_cmds.c:
> + else if (*p == '"') // comment start
+ break;
+ else if (eap->nextcmd == NULL && check_nextcmd(p) != NULL)
+ {
+ eap->nextcmd = check_nextcmd(p);
+ break;
+ }
+ else
+ {
+ semsg(_(e_invalid_argument_str), p);
+ goto uniqend;
+ }
+ }
+
+ /*
+ * Get the longest line length for allocating "sortbuf".
To be consistent with other comments, can you use "//" style comment here?
In src/ex_cmds.c:
> + s = ml_get(lnum);
+ len = ml_get_len(lnum);
+ if (maxlen < len)
+ maxlen = len;
+ }
+
+ // Allocate a buffer that can hold the longest line.
+ sortbuf1 = alloc(maxlen + 1);
+ if (sortbuf1 == NULL)
+ goto uniqend;
+
+ // Insert the lines in the sorted order below the last one.
+ lnum = eap->line2;
+ for (i = 0; i < count; ++i)
+ {
+ //linenr_T get_lnum = nrs[eap->forceit ? count - i - 1 : i].lnum;
This commented out line can be removed?
In src/ex_cmds.c:
> + if (i == 0 || string_compare(s, sortbuf1) != 0)
+ {
+ // Copy the line into a buffer, it may become invalid in
+ // ml_append(). And it's needed for "unique".
+ STRCPY(sortbuf1, s);
+ if (ml_append(lnum++, sortbuf1, (colnr_T)0, FALSE) == FAIL)
+ break;
+ }
+ fast_breakcheck();
+ if (got_int)
+ goto uniqend;
+ }
+
+ // delete the original lines if appending worked
+ if (i == count)
+ for (i = 0; i < count; ++i)
Can you move these inside curly braces?
> +
+" Test for retaining marks across a :uniq
+func Test_uniq_with_marks()
+ new
+ call setline(1, ['cc', 'cc', 'aa', 'bb', 'bb', 'bb', 'bb'])
+ call setpos("'c", [0, 1, 0, 0])
+ call setpos("'a", [0, 4, 0, 0])
+ call setpos("'b", [0, 7, 0, 0])
+ %uniq
+ call assert_equal(['cc', 'aa', 'bb'], getline(1, '$'))
+ call assert_equal(1, line("'c"))
+ call assert_equal(0, line("'a"))
+ call assert_equal(0, line("'b"))
+ close!
+endfunc
+
Can you also add a test for the undo after running ":uniq"?
> + + " Previously, the ":uniq" command would set 'modified' even if the buffer + " contents did not change. Here, we check that this problem is fixed. + if t.input == t.expected + call assert_false(&modified, t.name . ': &mod is not correct') + else + call assert_true(&modified, t.name . ': &mod is not correct') + endif + endfor + + " Needs at least two lines for this test + call setline(1, ['line1', 'line2']) + call assert_fails('uniq no', 'E475:') + call assert_fails('uniq c', 'E475:') + call assert_fails('uniq #pat%', 'E475:') + call assert_fails('uniq /\%(/', 'E475:')
Can you also add a test for supplying an invalid range (line numbers greater than the number of lines in the buffer) to the uniq command?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#17538)Hm, just a general comment first: What is the use-case for this? I can't remember having such a particular need in an editor.
If at all, I think it would be useful to specify an optional search argument (similar to the :sort command), so that one could specify a particular pattern on which to decide whether this is a duplicate. Think of handling CSV data and you want do deduplicate based on a particular column.
However, I am not so sure this should belong into core, it seems a vim plugin solution might be more flexible, no?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
zeertzjq
zeertzjq left a comment (vim/vim#17538)https://github.com/inkarkat/vim-AdvancedSorters seems to contain some related commands.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
h_east
@h-east commented on this pull request.
In src/ex_cmds.c:
> + if (i == 0 || string_compare(s, sortbuf1) != 0)
+ {
+ // Copy the line into a buffer, it may become invalid in
+ // ml_append(). And it's needed for "unique".
+ STRCPY(sortbuf1, s);
+ if (ml_append(lnum++, sortbuf1, (colnr_T)0, FALSE) == FAIL)
+ break;
+ }
+ fast_breakcheck();
+ if (got_int)
+ goto uniqend;
+ }
+
+ // delete the original lines if appending worked
+ if (i == count)
+ for (i = 0; i < count; ++i)
:uniq processing has been changed from m_append() --> m_delete() to only m_delete().
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
h_east
@h-east commented on this pull request.
> +
+" Test for retaining marks across a :uniq
+func Test_uniq_with_marks()
+ new
+ call setline(1, ['cc', 'cc', 'aa', 'bb', 'bb', 'bb', 'bb'])
+ call setpos("'c", [0, 1, 0, 0])
+ call setpos("'a", [0, 4, 0, 0])
+ call setpos("'b", [0, 7, 0, 0])
+ %uniq
+ call assert_equal(['cc', 'aa', 'bb'], getline(1, '$'))
+ call assert_equal(1, line("'c"))
+ call assert_equal(0, line("'a"))
+ call assert_equal(0, line("'b"))
+ close!
+endfunc
+
Added.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
h_east
@h-east pushed 1 commit.
- 4f1c384 fix review pointed out.
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
h_east
> + + " Previously, the ":uniq" command would set 'modified' even if the buffer + " contents did not change. Here, we check that this problem is fixed. + if t.input == t.expected + call assert_false(&modified, t.name . ': &mod is not correct') + else + call assert_true(&modified, t.name . ': &mod is not correct') + endif + endfor + + " Needs at least two lines for this test + call setline(1, ['line1', 'line2']) + call assert_fails('uniq no', 'E475:') + call assert_fails('uniq c', 'E475:') + call assert_fails('uniq #pat%', 'E475:') + call assert_fails('uniq /\%(/', 'E475:')
done
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
h_east
h-east left a comment (vim/vim#17538)Hm, just a general comment first: What is the use-case for this?
I needed to analyze the operation log of a certain device. When the state changes, the log is output in ASCII text, and when the state continues, the same log is output periodically. In this case, I use the Linux uniq command to combine the same lines while the state continues into one line, but I thought it would be nice if Vim could do that too, so I added :uniq.
I don't think it's used very often, but I think it would be nice if Vim itself had it.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#17538)okay fair enough. Can you make it similar to the :sort command, so that it is flexible enough what to consider as duplicates?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
h_east
h-east left a comment (vim/vim#17538)ping @yegappan
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
h_east
h-east left a comment (vim/vim#17538)@yegappan
I've seen you review PRs, mark them as requested changes, and then leave them without reviewing them again a few times. If you have some other purpose, you should be clear about it.
Also, I don't think the points raised in this review deserve to be marked as requested changes.
Anyway, I have responded to your review points.
I'd like to clear your requested changes before starting the following steps.
okay fair enough. Can you make it similar to the :sort command, so that it is flexible enough what to consider as duplicates?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Yegappan Lakshmanan
@yegappan approved this pull request.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Yegappan Lakshmanan
yegappan left a comment (vim/vim#17538)@yegappan I've seen you review PRs, mark them as
requested changes, and then leave them without reviewing them again a few times. If you have some other purpose, you should be clear about it. Also, I don't think the points raised in this review deserve to be marked asrequested changes.Anyway, I have responded to your review points. I'd like to clear your
requested changesbefore starting the following steps.okay fair enough. Can you make it similar to the :sort command, so that it is flexible enough what to consider as duplicates?
@h-east I have approved the change.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
h_east
h-east left a comment (vim/vim#17538)How about the following specifications?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Yegappan Lakshmanan
@yegappan commented on this pull request.
> @@ -1990,4 +1993,56 @@ The sorting can be interrupted, but if you interrupt it too late in the
process you may end up with duplicated lines. This also depends on the system
library function used.
+==============================================================================
+8. Deduplicating text *deduplicating* *unique*
+
+Vim has a deduplicating function and a deduplicating command. The
+deduplicating function can be found here: |uniq()|.
+Also see |:sort-uniq|.
+
+ *:uni* *:uniq*
+:[range]uni[q][!] [i][l][r][u] [/{pattern}/]
+ Remove duplicate lines that follow each other in
Suggestions from CoPilot for the help text:
:[range]uni[q][!][i][l][r][u] [/{pattern}/]
Remove consecutive duplicate lines in [range].
If no range is given, all lines are processed.
With [i], case is ignored when comparing lines.
With [l], comparison uses the current collation
locale. See |:sort-l| for more details.
With [r], only the portion of the line matching
/{pattern}/ is compared.
When /{pattern}/ is used without [r], the matching
text is skipped and comparison is done on what
follows it. 'ignorecase' applies to the pattern,
but 'smartcase' is not used.
Any non-letter character may be used in place of
the slash to delimit the pattern.
Examples:
Remove duplicates based on the second
comma-separated field: >
:uniq r /[^,]*,/
< Keep only lines unique after ignoring the
first 5 characters: >
:uniq u /.\{5}/
< If the pattern is empty (e.g. //), the last
search pattern is used.
With [u], only lines that do not repeat (i.e.,
not followed by the same line) are kept.
With [!], only lines that repeat (i.e., followed
by the same line) are kept.
If both [!] and [u] are given, [!] takes effect.
Note: lines must be identical and adjacent to be
considered duplicates. Leading and trailing white
space matters. To remove all duplicates regardless
of position, use |:sort-u| or external tools.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
h_east
@h-east commented on this pull request.
> @@ -1990,4 +1993,56 @@ The sorting can be interrupted, but if you interrupt it too late in the
process you may end up with duplicated lines. This also depends on the system
library function used.
+==============================================================================
+8. Deduplicating text *deduplicating* *unique*
+
+Vim has a deduplicating function and a deduplicating command. The
+deduplicating function can be found here: |uniq()|.
+Also see |:sort-uniq|.
+
+ *:uni* *:uniq*
+:[range]uni[q][!] [i][l][r][u] [/{pattern}/]
+ Remove duplicate lines that follow each other in
Thank you.
My help was also created by AI based on the help for :sort and corrected several times. This time, I will accept my help.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
h_east
h-east left a comment (vim/vim#17538)Can be merged.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Yegappan Lakshmanan
@yegappan commented on this pull request.
> @@ -1990,4 +1993,56 @@ The sorting can be interrupted, but if you interrupt it too late in the
process you may end up with duplicated lines. This also depends on the system
library function used.
+==============================================================================
+8. Deduplicating text *deduplicating* *unique*
+
+Vim has a deduplicating function and a deduplicating command. The
+deduplicating function can be found here: |uniq()|.
+Also see |:sort-uniq|.
+
+ *:uni* *:uniq*
+:[range]uni[q][!] [i][l][r][u] [/{pattern}/]
+ Remove duplicate lines that follow each other in
I think "Remove consecutive duplicate lines in" is more descriptive than "Remove duplicate lines that follow each other".
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
h_east
@h-east commented on this pull request.
> @@ -1990,4 +1993,56 @@ The sorting can be interrupted, but if you interrupt it too late in the
process you may end up with duplicated lines. This also depends on the system
library function used.
+==============================================================================
+8. Deduplicating text *deduplicating* *unique*
+
+Vim has a deduplicating function and a deduplicating command. The
+deduplicating function can be found here: |uniq()|.
+Also see |:sort-uniq|.
+
+ *:uni* *:uniq*
+:[range]uni[q][!] [i][l][r][u] [/{pattern}/]
+ Remove duplicate lines that follow each other in
The help has been overhauled again, and adjusted to use the word adjacent.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#17538)It seems, ssize_t is not defined on MS-Windows and therefore causes compile errors. Also there is a syntax error in your test.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
> + \ 'a', + \ 'à', + \ 'à', + \ 'E', + \ 'È', + \ 'É', + \ 'o', + \ 'O', + \ 'Ô', + \ 'e', + \ 'è', + \ 'é', + \ 'ô', + \ 'Œ', + \ 'œ', + \ 'z'⬇️ Suggested change
- \ 'z' + \ 'z',
> + \ 'à', + \ 'à', + \ 'E', + \ 'È', + \ 'É', + \ 'o', + \ 'O', + \ 'Ô', + \ 'e', + \ 'è', + \ 'é', + \ 'ô', + \ 'Œ', + \ 'œ', + \ 'z' + \ 'Z',⬇️ Suggested change
- \ 'Z', + \ 'Z'
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Yegappan Lakshmanan
@yegappan commented on this pull request.
> @@ -1990,4 +1993,56 @@ The sorting can be interrupted, but if you interrupt it too late in the
process you may end up with duplicated lines. This also depends on the system
library function used.
+==============================================================================
+8. Deduplicating text *deduplicating* *unique*
+
+Vim has a deduplicating function and a deduplicating command. The
+deduplicating function can be found here: |uniq()|.
+Also see |:sort-uniq|.
+
+ *:uni* *:uniq*
+:[range]uni[q][!] [i][l][r][u] [/{pattern}/]
+ Remove duplicate lines that follow each other in
The updated help text looks good to me.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
h_east
@h-east pushed 1 commit.
- 6ce3c40 Update src/testdir/test_uniq.vim
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#17538)Thanks, I changed the test to use :bw! instead of :close! so that now swap files are left around.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
dkearns
dkearns left a comment (vim/vim#17538)I just had an unexpected use for this @h-east, the day after it was merged. It was sufficiently faster than the alternatives that I wasn't sure it had actually worked at first. Nice work.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.