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

Bulk generate getter and setter #892

Merged
merged 4 commits into from
Dec 12, 2018

Conversation

testforstephen
Copy link
Contributor

Signed-off-by: Jinbo Wang [email protected]

Add a Source Action to bulk generate Getters and Setters for all possible fields.

Issues: #163, redhat-developer/vscode-java#597

@fbricon
Copy link
Contributor

fbricon commented Dec 10, 2018

the source action needs to have an id that can eventually be bindable by a key shortcut

@fbricon
Copy link
Contributor

fbricon commented Dec 10, 2018

The getter/setter code action should not be returned if 0 edits were computed.

Result: [
    {
        "title": "Generate Getters and Setters",
        "kind": "source",
        "diagnostics": [],
        "command": {
            "title": "Generate Getters and Setters",
            "command": "java.apply.workspaceEdit",
            "arguments": [
                {
                    "changes": {
                        "file:///Users/fbricon/Desktop/Standalones/Maths.java": []
                    }
                }
            ]
        }
    },
    {
        "title": "Organize imports",
        "kind": "source.organizeImports",
        "diagnostics": [],
        "command": {
            "title": "Organize imports",
            "command": "java.apply.workspaceEdit",
            "arguments": [
                {
                    "changes": {
                        "file:///Users/fbricon/Desktop/Standalones/Maths.java": []
                    }
                }
            ]
        }
    }

(same thing for organize imports BTW)

@fbricon
Copy link
Contributor

fbricon commented Dec 10, 2018

I opened #894 about the empty code actions. I'll provide a separate PR once I've written some tests

@testforstephen
Copy link
Contributor Author

@fbricon About the bindable id, do you mean we need add a new kind in lsp4j.CodeActionKind for bulk generate getter and setter?

@fbricon
Copy link
Contributor

fbricon commented Dec 11, 2018

Create a new kind in jdt.ls, whose prefix will be one of the lsp4j ones. Look if the Js/ts server has something similar, for inspiration

@testforstephen
Copy link
Contributor Author

There are only source, source.organizeImports in ts server. And looks like vscode doesn't recognize new customized source action kind, such as source.generateGetterSetter.

@fbricon
Copy link
Contributor

fbricon commented Dec 11, 2018

please squash your commits and ensure you fix the merge conflicts. Existing CodeActionHandlerTest tests should not be modified

rewrite.insertLast(declaration, null);
}

enum GENERATE_KIND {
Copy link
Contributor

Choose a reason for hiding this comment

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

don't use all caps for enum name

* Microsoft Corporation - initial API and implementation
*******************************************************************************/

package org.eclipse.jdt.ls.core.internal.corext.codemanipulation;
Copy link
Contributor

Choose a reason for hiding this comment

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

package org.eclipse.jdt.ls.core.internal.codemanipulation;

org.eclipse.jdt.ls.core.internal.corext.codemanipulation should only be used for code copied from jdt.ui

Copy link
Contributor

@fbricon fbricon left a comment

Choose a reason for hiding this comment

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

Note that source action key mapping is broken in VS Code.

}

ICompilationUnit unit = context.getCompilationUnit();
CUCorrectionProposal proposal = new CUCorrectionProposal(ActionMessages.GenerateGetterSetterAction_label, CodeActionKind.Source, unit, null, IProposalRelevance.GENERATE_GETTER_AND_SETTER) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Create a constant for jdt.ls specific kinds.
SOURCE_ACTION_GENERATE_KIND = CodeActionKind.Source+".generate";
SOURCE_ACTION_GENERATE_ACCESSORS_KIND = SOURCE_ACTION_GENERATE_KIND+".accessors";

Use SOURCE_ACTION_GENERATE_ACCESSORS_KIND

Copy link
Contributor

Choose a reason for hiding this comment

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

Kinds are hierarchical, so you need to also change ClientPreferences#isSupportedCodeActionKind so that it accepts sub kinds :

	public boolean isSupportedCodeActionKind(String kind) {
		//@formatter:off
		return v3supported && capabilities.getTextDocument().getCodeAction() != null
				&& capabilities.getTextDocument().getCodeAction().getCodeActionLiteralSupport() != null
				&& capabilities.getTextDocument().getCodeAction().getCodeActionLiteralSupport().getCodeActionKind() != null
				&& capabilities.getTextDocument().getCodeAction().getCodeActionLiteralSupport().getCodeActionKind().getValueSet() != null
				&& capabilities.getTextDocument().getCodeAction().getCodeActionLiteralSupport().getCodeActionKind().getValueSet()
				.stream().filter(k -> kind.startsWith(k)).findAny().isPresent();
		//@formatter:on
	}

@testforstephen
Copy link
Contributor Author

testforstephen commented Dec 12, 2018

@fbricon PR updated.

  • Squash commit.
  • Fix bindable key.
  • Address other comments.

* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-2.0/
Copy link
Contributor

Choose a reason for hiding this comment

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

So we're now copying EPL-2 code into a project that hasn't moved to EPL-2 yet (EPL-1 is deprecated, so we should do it eventually). I think this is fine, but would love to get confirmation about it from a professional (@waynebeaton ?)

@fbricon fbricon merged commit b25d8ec into eclipse-jdtls:master Dec 12, 2018
@testforstephen testforstephen deleted the jinbo_bulkgettersetter branch December 13, 2018 01:36
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.

2 participants