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.

Sign up for GitHub

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

Jump to bottom

Adds a 'format to camelCase' codeaction for function names #679

Merged
merged 10 commits into from
Sep 30, 2022
Merged

Adds a 'format to camelCase' codeaction for function names #679

merged 10 commits into from
Sep 30, 2022

Conversation

NuclearPhone
Copy link
Contributor

Does as the title says, this PR adds a code action that reformats non-camelCase function names to be camelCase.

camelcase1
camelcase2

Techatrix
Techatrix requested changes Sep 28, 2022
View reviewed changes
Copy link
Member

@Techatrix Techatrix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for you PR! It's great to see more code actions getting integrated. There are "a few" issues though.
If you are interested you can also add code actions for Type functions should be PascalCase or A ./ is not needed in imports.

src/code_actions.zig Outdated
@@ -159,7 +188,7 @@ fn handleUnusedVariableOrConstant(builder: *Builder, actions: *std.ArrayListUnma

try actions.append(builder.arena.allocator(), .{
.title = "discard value",
.kind = .SourceFixAll,
.kind = .QuickFix,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be keep at SourceFixAll

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this has to be resolved before i can approve

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, i'm not too familiar with git and i missed this. fixed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think your latest commit touched the wrong place.
Set this location https://github.com/NuclearPhone/zls/blob/39ea5869a377eeec71fe333f2ee264fac7cf2bbb/src/code_actions.zig#L85 to .QuickFix.
And set this location https://github.com/NuclearPhone/zls/blob/39ea5869a377eeec71fe333f2ee264fac7cf2bbb/src/code_actions.zig#L176 to .SourceFixAll.

Copy link
Contributor Author

@NuclearPhone NuclearPhone Sep 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed.

src/code_actions.zig Outdated Show resolved Hide resolved
src/code_actions.zig Outdated Show resolved Hide resolved
src/code_actions.zig Outdated Show resolved Hide resolved
src/code_actions.zig Show resolved Hide resolved
src/code_actions.zig Outdated Show resolved Hide resolved
@NuclearPhone
Copy link
Contributor Author

Thanks for the feedback; I'll get to fixing my error-ridden code. In the future i'll pay more attention to proof-reading.

Techatrix
Techatrix approved these changes Sep 29, 2022
View reviewed changes
Copy link
Member

@Techatrix Techatrix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

@SuperAuguste SuperAuguste merged commit a8fa5c6 into zigtools:master Sep 30, 2022
@Techatrix Techatrix mentioned this pull request Jan 23, 2023
Open
24 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Techatrix Techatrix Techatrix approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@NuclearPhone @Techatrix @SuperAuguste