Page MenuHomePhabricator

[clangd] Place cursor better after completing patterns
ClosedPublic

Authored by ilya-biryukov on May 24 2019, 6:54 AM.

Details

Summary

By producing the $0 marker in the snippets at the last placeholder.
This produces nicer results in most cases, e.g. for

namespace <#name#> {
  <#decls#>
}

we now produce ${0:decls} instead of ${2:decls} and the final cursor
placement is more convenient.

Diff Detail

Repository
rL LLVM

Event Timeline

ilya-biryukov created this revision.May 24 2019, 6:54 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 24 2019, 6:54 AM

I'm not sure the new behavior suits all cases, it seems to me that this will change a typical behavior (usually when users invoke function calls).

Imaging a case:

void foo(int a, int b);

void call() {
  fo^
}
  • before, the final cursor is at the end of the snippet
  • after, the final cursor is at the last parameter of foo.

I think the previous cursor location is more convenient (user can continue typing ;).

clang-tools-extra/clangd/CodeCompletionStrings.h
41 ↗(On Diff #201227)

IIUC, I think we will still return ${0} with the new behavior, size_type is the only placeholder here.

I'm not sure the new behavior suits all cases, it seems to me that this will change a typical behavior (usually when users invoke function calls).

Imaging a case:

void foo(int a, int b);

void call() {
  fo^
}
  • before, the final cursor is at the end of the snippet
  • after, the final cursor is at the last parameter of foo.

    I think the previous cursor location is more convenient (user can continue typing ;).

Agree! The new behavior does not apply to function calls, they are not considered 'patterns' in the completion results.
I.e. in your examples we're still getting foo(${1:int a}, ${2:int b}) and the cursor is placed at the end.

This is definitely worth a comment, I'll try to find a good place for it.

ilya-biryukov marked an inline comment as done.May 27 2019, 2:35 AM
ilya-biryukov added inline comments.
clang-tools-extra/clangd/CodeCompletionStrings.h
41 ↗(On Diff #201227)

The comment was incorrect, we did not return ${0:} placeholders before.
Note that behavior for function call arguments is the same in both the old and the new version.

I'm not sure the new behavior suits all cases, it seems to me that this will change a typical behavior (usually when users invoke function calls).

Imaging a case:

void foo(int a, int b);

void call() {
  fo^
}
  • before, the final cursor is at the end of the snippet
  • after, the final cursor is at the last parameter of foo.

    I think the previous cursor location is more convenient (user can continue typing ;).

Agree! The new behavior does not apply to function calls, they are not considered 'patterns' in the completion results.
I.e. in your examples we're still getting foo(${1:int a}, ${2:int b}) and the cursor is placed at the end.

This is definitely worth a comment, I'll try to find a good place for it.

Ah, that makes sense! Thanks for clarifying it.

How about adding one more test in CodeCompleteTests.cpp, we already have a snippet test there, and it seems more straightforward.

  • Add a code completion test

How about adding one more test in CodeCompleteTests.cpp, we already have a snippet test there, and it seems more straightforward.

Done!

hokein accepted this revision.May 27 2019, 6:57 AM

LGTM

This revision is now accepted and ready to land.May 27 2019, 6:57 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 28 2019, 8:31 AM