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

Improve "extract function" dialog #2186

Open
7 of 8 tasks
Undin opened this issue Jan 10, 2018 · 1 comment
Open
7 of 8 tasks

Improve "extract function" dialog #2186

Undin opened this issue Jan 10, 2018 · 1 comment
Labels
E-unknown Issue where level of knowledge about platform and project codebase is currently unknown help wanted improvement subsystem::code insight General label for issues related to code understanding: highlighting, completion, annotation, etc. subsystem::ui Any problems in user interface

Comments

@Undin
Copy link
Member

Undin commented Jan 10, 2018 •
edited by Kobzol
Loading

Current "extract function" dialog is not ideal and can be improved:

  • Don't allow press "OK" button if function name is empty (it leads to exception) or invalid
  • Show simple text field if we can't suggest function name instead of empty drop down list
  • Reformat code in "Signature" field (and maybe drop function body at all)
  • Enable window resizing
  • Allow renaming arguments
  • Allow to manually select arguments
  • Allow changing argument's ownership/mutability (T/&T/&mut T/mut T)
  • Allow changing argument's type

Good example is kotlin "Extract function" dialog
screen shot 2018-01-11 at 00 14 55
@Undin Undin added help wanted E-unknown Issue where level of knowledge about platform and project codebase is currently unknown subsystem::code insight General label for issues related to code understanding: highlighting, completion, annotation, etc. improvement subsystem::ui Any problems in user interface labels Jan 10, 2018
@Undin Undin mentioned this issue Jan 10, 2018
Open
20 tasks
@Kobzol Kobzol mentioned this issue May 10, 2019
Merged
bors bot added a commit that referenced this issue May 20, 2019
@bors @Kobzol
Merge #3827
84ba447 
3827: REF: improve extract method dialog UI r=ortem a=Kobzol

This PR attempts to improve the Extract method dialog UI.
Related issues: #2186, #2183

Right now it fixes an exception that was thrown when an invalid name was used for the name of the function (#1934).

I also tried to use a simple text field instead of the dropdown list if there are no suggestions to offer, however `NameSuggestionsField` always returns true from `hasSuggestions()` method (which is caused by the way it's used in the dialog), so I'm not sure how to recognize when there are no suggestions (maybe use a different component altogether)?

Co-authored-by: Jakub Beránek <berykubik@gmail.com>
Stzx added a commit to Stzx/intellij-rust that referenced this issue Nov 11, 2019
@Stzx
Allow to manually select arguments intellij-rust#2186
56dd705
Stzx added a commit to Stzx/intellij-rust that referenced this issue Nov 11, 2019
@Stzx
Allow to manually select arguments intellij-rust#2186
fc4a97c
Stzx added a commit to Stzx/intellij-rust that referenced this issue Nov 12, 2019
@Stzx
Allow to manually select arguments intellij-rust#2186
0c2f6f0
@Stzx Stzx mentioned this issue Nov 14, 2019
Merged
Stzx added a commit to Stzx/intellij-rust that referenced this issue Nov 28, 2019
@Stzx
Allow to manually select arguments intellij-rust#2186
ac15e5b
Stzx added a commit to Stzx/intellij-rust that referenced this issue Dec 3, 2019
@Stzx
Allow to manually select arguments intellij-rust#2186
e614648
bors bot added a commit that referenced this issue Dec 4, 2019
@bors @Stzx
Merge #4619
af8995e 
4619: Allow to manually select arguments r=ortem a=Stzx

:)

Edited: relates to #2186

Co-authored-by: Stzx <silence.m@hotmail.com>
@Kobzol Kobzol mentioned this issue Apr 30, 2020
Merged
bors bot added a commit that referenced this issue Jun 15, 2020
@bors @Kobzol
Merge #5313
bdf38ed 
5313: REF: allow the user to select mutability of parameters in ExtractFunction r=mchernyavsky a=Kobzol

This PR adds mutability selection to the UI of `ExtractFunction`. I separated mutability from references, which should make it easier to add another option to choose "by value" or "by ref" parameters in the future.

There is a slightly weird interaction with `requiresMut` (formerly called `isMutableValue`), for example here:
```rust
fn test(mut v: Vec<i32>) {}
fn test2(v: &mut Vec<i32>) {}

fn foo() {
    let mut vec = vec![1, 2, 3];
    let mut vec2 = vec![1, 2, 3];

    /*caret*/bar(vec, &mut vec2);
}

fn bar(mut vec: Vec<i32>, mut vec2: &mut Vec<i32>) {
    test(vec);
    test2(&mut vec2);
}
```

If I turn off the mutability of `vec2`, it switches from `mut vec2: &mut Vec<i32>` to `vec2: &Vec<i32>`. A better solution would be to not require `mut` there and just pass `vec2` to `test2` directly: `test2(vec2)`. But that would require further changes in how the function is generated, as the extracted code would have to be modified.

Umbrella issue: #2186

Co-authored-by: Jakub Beránek <berykubik@gmail.com>
@Kobzol
Copy link
Member

Kobzol commented Jun 15, 2020

Should we also support the last item on the list, Allow changing argument's type?

For example, the Kotlin plugin is pretty restrictive and doesn't allow type changes apart from subtyping. It also disallows removing the parameters.

We already allow removing the parameter though, so maybe we should also allow changing the type. It shouldn't be hard to implement, we could just allow the user to type in the new type, although it's pretty easy to break the extracted code with that. I will try to implement it if you think that it's a valid option to offer to the users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
E-unknown Issue where level of knowledge about platform and project codebase is currently unknown help wanted improvement subsystem::code insight General label for issues related to code understanding: highlighting, completion, annotation, etc. subsystem::ui Any problems in user interface
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Undin @Kobzol