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 argument placeholder #1317

Merged
merged 4 commits into from
Jul 16, 2023
Merged

Conversation

jiacai2050
Copy link
Contributor

@jiacai2050 jiacai2050 commented Jul 12, 2023

Close #685

@leecannon
Copy link
Member

You need to add the option to src/config_gen/config.json then run zig build gen.

Also the default should be true to match the current behaviour.

@jiacai2050
Copy link
Contributor Author

PTAL

@leecannon
Copy link
Member

This isn't working correctly.

Below is the result of completion of std.debug.print, the text is what is actually inserted into the editor:

enable_argument_placeholders true enable_argument_placeholders false
enable_snippets true std.debug.print(comptime fmt: []const u8, args: anytype) std.debug.print(${})
enable_snippets false std.debug.print(${1:comptime fmt: []const u8}, ${2:args: anytype}) std.debug.print(${})

Example:
image

This is what should be inserted into the editor:

enable_argument_placeholders true enable_argument_placeholders false
enable_snippets true std.debug.print(comptime fmt: []const u8, args: anytype) std.debug.print()
enable_snippets false std.debug.print std.debug.print

So the only combination working as expected is when both are true.

@jiacai2050
Copy link
Contributor Author

jiacai2050 commented Jul 14, 2023

Ooops, I have make changes to make result right, but I have one question,

when enable_snippets = true, enable_argument_placeholders = false, if we output std.debug.print(), cursor will be placed after )

If we output std.debug.print(${}), cursor will be placed after (, which is more reasonable to me.

@leecannon
Copy link
Member

leecannon commented Jul 14, 2023

Correct, with snippets but not argument placeholders it should be std.debug.print(^) (^ used to signify cursor location after accepting completion) but what was happening was std.debug.print(${})^ with the ${} actually inserted into the document and not handled specially by the editor.

I haven't check the behaviour of the latest commit, as I'm still at work and should not even being looking at this right now :),

@leecannon
Copy link
Member

I've added a commit which ensures the placement of the cursor is correct when argument placeholders are disabled but snippets are enabled.

This PR is looking good to merge however it won't be merged until #1311 is to prevent more conflicts building up on that PR, don't worry though it should only be a couple of days.

jiacai2050 and others added 4 commits July 16, 2023 14:01
Correctly handles cursor placement after a completion when placeholders are disabled but snippets are not.
@leecannon leecannon merged commit 73033c3 into zigtools:master Jul 16, 2023
Techatrix pushed a commit that referenced this pull request Aug 26, 2023
* support argument placeholder

* default to true

* fix review

* completion: handle cursor placement

Correctly handles cursor placement after a completion when placeholders are disabled but snippets are not.

---------

Co-authored-by: Lee Cannon <leecannon@leecannon.xyz>
KoltPenny pushed a commit to KoltPenny/zls that referenced this pull request Oct 18, 2023
* support argument placeholder

* default to true

* fix review

* completion: handle cursor placement

Correctly handles cursor placement after a completion when placeholders are disabled but snippets are not.

---------

Co-authored-by: Lee Cannon <leecannon@leecannon.xyz>
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.

Feature flag request: Separate argument injection from snippets
2 participants