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

Missing documentation on refactoring & completion #82

Open
wusatosi opened this issue May 1, 2023 · 13 comments
Open

Missing documentation on refactoring & completion #82

wusatosi opened this issue May 1, 2023 · 13 comments
Assignees

Comments

@wusatosi
Copy link

wusatosi commented May 1, 2023

Currently, only rename is mentioned under the features tab.

Should we include other implemented refactoring features like inline/ extract variables?
Related: clangd/clangd#446 (comment)

Besides inline/ extract variables, refactoring functionalities under clangd/refactor/tweaks is barely documented.

Is this intended?

@wusatosi wusatosi changed the title Missing refactoring functionalities Missing documentation on refactoring & completion May 1, 2023
@wusatosi
Copy link
Author

wusatosi commented May 1, 2023

May also include completion on enum based switches. See clangd/clangd#807 .

@HighCommander4
Copy link
Contributor

I think it would be valuable to document all refactorings / tweaks.

Are you interested in writing a documentation patch?

@wusatosi
Copy link
Author

wusatosi commented May 2, 2023

I think it would be valuable to document all refactorings / tweaks.

Are you interested in writing a documentation patch?

That will be fun. Can we collect a comprehensive list of refactoring support and tweaks for me to write up? Also, what should be noted upon refactoring.

The list:

  • Expand auto [since 9, commit]
  • Extract variable [since 9, commit]
  • Expand Macro [since 9, commit]
  • Raw string literal [since 9, commit]
  • Remove using name space [since 10, commit]
  • Extract functions [since 10, commit]
  • Define Inline, Outline [since 10, commit]
  • What is this "ObjCLocalizeStringLiteral"? [since 10, commit]
  • AddUsing [since 11, commit]
  • Auto completion of enum-based switch statement [since 12, commit]
  • Member wise construct [since 15, commit]
  • Generate move/copy constrct/assignment func [since 15, commit]
  • ObjC initializer [since 15, commit]
  • Expand deduced type [since 16, commit]

Hidden tweaks

  • Annotate Highlight
  • Switch if
  • Dump AST [since 9, commit]

Improvements

  • Expand deduced type [since 16, commit]

Edit: please assign this issue to me.

Edit 2: @HighCommander4 Can you double check the above list of functionalities for me to see if there's anything missing? And if they are on the right "since" version. Also, since we haven't released version 16 yet (clangd/clangd#1567) should I include the "Expand Deduced type"?

@HighCommander4
Copy link
Contributor

HighCommander4 commented May 2, 2023

Are you interested in writing a documentation patch?

That will be fun.

Thanks for your interest!

Can we collect a comprehensive list of refactoring support and tweaks for me to write up?

I think the authoritative list here is the source:

https://searchfox.org/llvm/source/clang-tools-extra/clangd/refactor/tweaks

As you can see, there are quite a few. Feel free to cover just a subset of them initially and leave others for later, if that makes things easier.

The tests are also helpful to get a feel for what each refactoring can do:

https://searchfox.org/llvm/source/clang-tools-extra/clangd/unittests/tweaks

Also, what should be noted upon refactoring.

I think a 1-sentence description and one before/after code example for each tweak would be a good start.

Another thing that would be valuable to document are significant limitations of a given refactoring, for example the fact that "extract function" does not currently support extracting a single expression-statement as dicussed in clangd/clangd#1254, however this is often not obvious without more in-depth knowledge of the refactoring (to me either), so we can probably leave this for future additions.

@HighCommander4
Copy link
Contributor

HighCommander4 commented May 2, 2023

Edit: please assign this issue to me.

I don't have the permission to do that in this repository (it's not in the clangd org). Let's ask @kadircet.

@kadircet
Copy link
Member

kadircet commented May 2, 2023

As for tweak documentation, we usually have a good description of what they do, sometimes even with examples, as class documentation. See https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp#L35.

@kadircet
Copy link
Member

kadircet commented May 2, 2023

Also thanks for doing this!

@wusatosi
Copy link
Author

wusatosi commented May 2, 2023

As for tweak documentation, we usually have a good description of what they do, sometimes even with examples, as class documentation. See https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp#L35.

Thanks for the tip!

@HighCommander4
Copy link
Contributor

HighCommander4 commented May 3, 2023

This one is hidden.

  • What is this "ObjCLocalizeStringLiteral"? [since 10, commit]

It sounds like this is a tweak for wrapping an objective-C string literal into an NSLocalizedString() macro, which I guess calls some sort of library facility to localize (translate) the string into the language of the application's locale.

  • Generate move/copy constrct/assignment func [since 15, commit]

(You linked to "remove using namespace" here, but I believe you're right about "generate move/copy..." being added in clangd 15.)

  • Expand deduced type [since 16, commit]

This one is actually an improvement of the "expand auto type" code action originally added in clangd 9, to also support decltype(expr). We can mention both the original auto support and the improvement.

Otherwise your list looks good to me!

@HighCommander4
Copy link
Contributor

HighCommander4 commented May 3, 2023

There are two more, DefineInline and DefineOutline.

@wusatosi
Copy link
Author

wusatosi commented May 3, 2023

There are two more, DefineInline and DefineOutline.

nice catch!

@wusatosi
Copy link
Author

wusatosi commented May 3, 2023

Proposed classification:

Code completion

Populate switch statement [since 12]
Generate member wise constructor [since 15, also ObjC]
Generate move/copy constrct/assignment func [since 15]

Refactor

Remove using name space [since 10]
Extract namespace as using [since 11]
Expand auto declaration [since 9, decltype since 16]
Expand Macro [since 9]
Extract variable [since 9]
Extract functions [since 10]
Convert to raw string literal [since 9]
Define Inline, Outline [since 10]
Warp in NSLocalizedString [since 10, objective-c only]

@wusatosi
Copy link
Author

wusatosi commented May 3, 2023

You can track my progress here:
https://main--clinquant-entremet-9c06af.netlify.app/features

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

No branches or pull requests

3 participants