feat!: Pattern/function-defined modifications. #113

kylechui · 2022-07-26T21:05:18Z

No description provided.

* TODO: Write translation layer.

* For better accuracy/to avoid edge cases, probably use `it/at`.

smjonas · 2022-07-28T07:18:38Z

lua/nvim-surround/config.lua

-            ["["] = { "[ ", " ]" },
-            ["]"] = { "[", "]" },
-            ["i"] = function()
+        ["("] = {


Will it be possible to simply set ( (or maybe the add key) to a table of characters here? While that's basically just syntactic sugar, I think that makes the API look a bit nicer for simple cases like this.

The default configuration has to "look complicated" (unless I translate the default on every run, which seems non-ideal), since it uses the internal representation that every function in the program assumes exists. That being said, user configuration will be simpler, as I will write a "translation" function that will convert user-provided configuration into the internal form, and then merge to provide the proper feature set. I think I plan on the add key being a table of characters, as it would provide a better "mix-and-match" for syntactic sugar between add/delete/change functions instead of tying everything together into an "all or nothing".

Initial work has been done; if the surrounding pair is "static" then you may provide a pair instead of a function. Furthermore, strings will be auto-converted to tables with one string (i.e. multi-line form). For example, I have this in my ftplugin/lua.lua for printing (via yssp):

["p"] = { add = { "vim.pretty_print(", ")" }, },

smjonas · 2022-08-03T20:20:12Z

lua/nvim-surround/config.lua

+    if not (opts and opts.delimiters) then
+        return opts
+    end
+    local invalid = opts.delimiters.invalid_key_behavior or M.default_opts.delimiters.invalid_key_behavior


Could you maybe add another deprecation warning for invalid_key_behavior? (there was an error for me because I still had it set to a function). Something like this:

if type(invalid) == "function" then local invalid_key_warning = { "The old way of configuring `invalid_key_behavior` has been deprecated;", "see :h nvim-surround.config.delimiters for details.", } vim.notify_once(table.concat(invalid_key_warning, "\n"), vim.log.levels.WARN) end

smjonas · 2022-08-03T20:24:50Z

doc/nvim-surround.txt

+
+There is a special key in the `delimiters` table, `invalid_key_behavior`, that
+defines how |nvim-surround| behaves when a given key in the delimiter table is
+omitted. It takes the same form as all other delimiter tables. The default


Personally, I think a better default is to simply delete / change the character specified. At least that's what I would expect as a user coming from other surround plugins. Curious what other users think here (cc @andrewferrier, @jonatan-branting, @akinsho).

For some reason I was under the impression that vim-surround NOOP'd for everything; I've personally been using a setup that does just that. I'll probably change the default behavior for this then.

Changed by 502a430.

I just tested my claim with vim-sandwich and vim-surround and I must say I wasn't completely correct, sorry! vim-sandwich works as I have described but vim-surround doesn't: e.g. for dsx nothing happens but for non-letter characters such as * the characters are deleted. There seem to be special cases though such as:

*word*, action: ds* => result: word

* word *

action ds* => result: nothing changed

I think the current setup is easy enough to understand; I don't really want to bother with adding more special cases to handle everything. I don't think it's that hard to disable the built-in behavior and add your own keys into the delimiter table for *, $, -, etc.

That's fine, just pointing out the difference to existing plugins because it seemed like you agreed that you want to match their behavior :) I don't think it makes sense to try to match their behavior exactly if it makes things too complicated.

Yes of course, I always appreciate extra feedback. Also thank you so much for taking the time to review the code! I still haven't gotten around to writing a short blurb that introduces the code structure, so I apologize for that 😄

smjonas · 2022-08-03T20:38:25Z

Just noticed I get an error now when I run css / dss on a line:

E5108: Error executing lua ...k/packer/start/nvim-surround/lua/nvim-surround/utils.lua:100: start is higher than end                                                                                                                    
stack traceback:                                                                                                                                                                                                                        
        [C]: in function 'get_text'                                                                                                                                                                                                     
        ...k/packer/start/nvim-surround/lua/nvim-surround/utils.lua:100: in function 'target'                                                                                                                                           
        ...k/packer/start/nvim-surround/lua/nvim-surround/utils.lua:142: in function 'get_nearest_selections'                                                                                                                           
        ...ck/packer/start/nvim-surround/lua/nvim-surround/init.lua:270: in function <...ck/packer/start/nvim-surround/lua/nvim-surround/init.lua:263>

I also get an error with dsq:

E5108: Error executing lua ...acker/start/nvim-surround/lua/nvim-surround/patterns.lua:115: attempt to perform arithmetic on local 'first_index' (a nil value)                                                                          
stack traceback:                                                                                                                                                                                                                        
        ...acker/start/nvim-surround/lua/nvim-surround/patterns.lua:115: in function 'get_selections'                                                                                                                                   
        ...k/packer/start/nvim-surround/lua/nvim-surround/utils.lua:98: in function <...k/packer/start/nvim-surround/lua/nvim-surround/utils.lua:81>                                                                                    
        ...k/packer/start/nvim-surround/lua/nvim-surround/utils.lua:142: in function 'get_nearest_selections'                                                                                                                           
        ...ck/packer/start/nvim-surround/lua/nvim-surround/init.lua:147: in function 'delete_surround'                                                                                                                                  
        ...ck/packer/start/nvim-surround/lua/nvim-surround/init.lua:257: in function <...ck/packer/start/nvim-surround/lua/nvim-surround/init.lua:248>

kylechui · 2022-08-03T20:47:17Z

Just noticed I get an error now when I run css / dss on a line:

E5108: Error executing lua ...k/packer/start/nvim-surround/lua/nvim-surround/utils.lua:100: start is higher than end                                                                                                                    
stack traceback:                                                                                                                                                                                                                        
        [C]: in function 'get_text'                                                                                                                                                                                                     
        ...k/packer/start/nvim-surround/lua/nvim-surround/utils.lua:100: in function 'target'                                                                                                                                           
        ...k/packer/start/nvim-surround/lua/nvim-surround/utils.lua:142: in function 'get_nearest_selections'                                                                                                                           
        ...ck/packer/start/nvim-surround/lua/nvim-surround/init.lua:270: in function <...ck/packer/start/nvim-surround/lua/nvim-surround/init.lua:263>

I also get an error with dsq:

E5108: Error executing lua ...acker/start/nvim-surround/lua/nvim-surround/patterns.lua:115: attempt to perform arithmetic on local 'first_index' (a nil value)                                                                          
stack traceback:                                                                                                                                                                                                                        
        ...acker/start/nvim-surround/lua/nvim-surround/patterns.lua:115: in function 'get_selections'                                                                                                                                   
        ...k/packer/start/nvim-surround/lua/nvim-surround/utils.lua:98: in function <...k/packer/start/nvim-surround/lua/nvim-surround/utils.lua:81>                                                                                    
        ...k/packer/start/nvim-surround/lua/nvim-surround/utils.lua:142: in function 'get_nearest_selections'                                                                                                                           
        ...ck/packer/start/nvim-surround/lua/nvim-surround/init.lua:147: in function 'delete_surround'                                                                                                                                  
        ...ck/packer/start/nvim-surround/lua/nvim-surround/init.lua:257: in function <...ck/packer/start/nvim-surround/lua/nvim-surround/init.lua:248>

Do you mind reporting this here, and providing a buffer to re-create it? Also make sure to update to the latest version of the plugin, thanks!

smjonas · 2022-08-04T16:55:24Z

lua/nvim-surround/patterns.lua

+---@return selections @The selections for the left and right delimiters.
+M.get_selections = function(offset, str, pattern)
+    -- Get the surrounding pair, and the start/end indices
+    local _, _, left_delimiter, first_index, right_delimiter, last_index = str:find(pattern)


That looks problematic to me: if str is an empty string, first_index and last_index will always be nil so adding to it will fail. There should be a check that str:find actually found something.

Also, I think you should handle the case when pattern only contains one pair of () instead of the expected two (if that's not already done elsewhere).

That's a good point; I should be handling errors here in case of an invalid Lua pattern. With respect to your first point, first_index and last_index can potentially be empty strings instead of nil, but I agree that error-handling should be done to ensure that the program doesn't arbitrarily crash.

As for your second point, there are actually four sets of () required, see :h nvim-surround.config.get_selections(). I think it would make sense here to put a warning about it, rather than NOOP-ing?

Yes I agree, a warning definitely makes sense here.

Introduced this in 12c0338 (but update to the most recent one since I broke tests), if you would like to try it out. I'm going to re-read through the help docs/code to see if I can find any more improper uses of the words surrounds/delimiters

* Fix a few left-over places where `delimiters` is still used. * Add deprecation warning about using `surrounds` for configuration.

* When querying the user for input, the column gets clamped to the nearest valid column, i.e. #line. This fix grabs the cursor position before clamping occurs, allowing for proper insertion at the end of the line.

kylechui added 16 commits July 23, 2022 15:51

refactor!: Restructure user configuration.

613e9fe

refactor!: Use strict typing for configuration.

c6baff5

* TODO: Write translation layer.

feat: Some semblance of a working prototype.

b7f3c20

fix: Indexing issue, edge case with first line.

b19a5a4

fix: Reverse search patterns.

f587588

chore!: Remove arguments from function calls.

af3ec2c

fix: Quote configuration, non-pattern based finds.

5975c44

fix: Get HTML tags (mostly) working.

92a92c4

* For better accuracy/to avoid edge cases, probably use `it/at`.

fix: Smart quotes, somehow passes all test cases.

2a19c9b

feat: Add pattern-based deletions.

00aabd9

fix: Find function error.

1029204

fix: Deletions not occurring on same line.

63b5279

feat: Enable change.replacement omission.

393ec4d

docs: Better comment the code, introduce config helpers.

512aae9

docs: Update type annotations.

7d34e24

fix: Deletion error when setting up custom ds".

56e56a0

This was referenced Jul 26, 2022

dot repeat restores wrong cursor position #112

Closed

delete surrounding function scope / leave function body #114

Closed

kylechui linked an issue Jul 26, 2022 that may be closed by this pull request

Custom deletions based on patterns/Tree-sitter #60

Closed

1 task

kylechui added 4 commits July 27, 2022 11:38

refactor!: Move invalid_key_behavior back in delimiters.

cdd3255

fix: Some HTML Lua patterns.

44bf858

chore: Minor changes (patterns, annotations, etc.).

69b97e9

refactor: Omit requirement for modify pattern.

c013dfd

smjonas reviewed Jul 28, 2022

View reviewed changes

feat: Initial translation layer.

686ad38

kylechui force-pushed the pattern-matching branch from f407eac to 686ad38 Compare July 28, 2022 13:34

kylechui added 4 commits July 28, 2022 08:37

feat: Finish translation layer.

6b9fe8d

style: Use translation layer on default configuration.

d5dff6a

style: Use translation layer for more config.

7f2a38d

fix: Aliasing issues.

6f2cbd8

kylechui added 2 commits August 2, 2022 15:13

feat: Prefer surrounds that start near the cursor.

e3d449f

fix: Re-enable aliases (oops).

366e83a

kylechui force-pushed the pattern-matching branch from 6bc0487 to 366e83a Compare August 2, 2022 22:50

kylechui added 2 commits August 3, 2022 09:45

feat: Add soft deprecation warnings.

97b14f1

tests: Rewrite tests.

9b863e2

kylechui force-pushed the pattern-matching branch from e73695a to 9b863e2 Compare August 3, 2022 17:47

kylechui added 3 commits August 3, 2022 11:04

docs: Link to delimiters section as necessary.

da9ec56

docs: Add i key description.

735254e

docs: Update README.

eee6d21

smjonas reviewed Aug 3, 2022

View reviewed changes

kylechui added 2 commits August 3, 2022 13:59

feat!: Change default invalid_key_behavior.

502a430

refactor!: Rename highlight_motion to highlight.

0bf663a

smjonas reviewed Aug 4, 2022

View reviewed changes

kylechui added 12 commits August 4, 2022 10:02

refactor!: Use term surrounds for configuration.

8ce3075

chore: Validate inputs for pattern-matching.

12c0338

* Fix a few left-over places where `delimiters` is still used. * Add deprecation warning about using `surrounds` for configuration.

fix: Type-casting to integer.

a8a156c

docs: Link to external resources.

a393dbc

fix: Blockwise visual map with gS.

8397695

tests: Test open/close whitespace.

bbf0d5b

docs: Clean up help docs, remove more delimiters.

6bb79db

feat: Use nightly cancelreturn in get_input().

68b6117

fix: Incompatibility with targets.vim.

f6ac5f3

fix: Insert surrounds on last column.

6e5d744

* When querying the user for input, the column gets clamped to the nearest valid column, i.e. #line. This fix grabs the cursor position before clamping occurs, allowing for proper insertion at the end of the line.

style: Simplify default configuration.

e308640

fix: Invalid key behavior for single characters.

846e310

kylechui merged commit a06dea1 into main Aug 5, 2022

kylechui mentioned this pull request Aug 5, 2022

Breaking Changes: Following main #77

Open

kylechui deleted the pattern-matching branch August 6, 2022 03:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: Pattern/function-defined modifications. #113

feat!: Pattern/function-defined modifications. #113

kylechui commented Jul 26, 2022

smjonas Jul 28, 2022 •

edited

Loading

kylechui Jul 28, 2022 •

edited

Loading

kylechui Jul 28, 2022

smjonas Aug 3, 2022

smjonas Aug 3, 2022

kylechui Aug 3, 2022

kylechui Aug 3, 2022

smjonas Aug 3, 2022

kylechui Aug 3, 2022

smjonas Aug 4, 2022

kylechui Aug 4, 2022

smjonas commented Aug 3, 2022 •

edited

Loading

kylechui commented Aug 3, 2022

smjonas Aug 4, 2022 •

edited

Loading

kylechui Aug 4, 2022

smjonas Aug 4, 2022

kylechui Aug 4, 2022 •

edited

Loading

feat!: Pattern/function-defined modifications. #113

feat!: Pattern/function-defined modifications. #113

Conversation

kylechui commented Jul 26, 2022

smjonas Jul 28, 2022 • edited Loading

Choose a reason for hiding this comment

kylechui Jul 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smjonas commented Aug 3, 2022 • edited Loading

kylechui commented Aug 3, 2022

smjonas Aug 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kylechui Aug 4, 2022 • edited Loading

Choose a reason for hiding this comment

smjonas Jul 28, 2022 •

edited

Loading

kylechui Jul 28, 2022 •

edited

Loading

smjonas commented Aug 3, 2022 •

edited

Loading

smjonas Aug 4, 2022 •

edited

Loading

kylechui Aug 4, 2022 •

edited

Loading