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

Add add_to_top_level_lambda_param assist #41

Merged
merged 1 commit into from
Dec 5, 2022
Merged

Add add_to_top_level_lambda_param assist #41

merged 1 commit into from
Dec 5, 2022

Conversation

figsoda
Copy link
Contributor

@figsoda figsoda commented Dec 4, 2022

suggests { }: foo -> { foo }: foo

related issue: #39

Copy link
Owner

@oxalica oxalica left a comment

Choose a reason for hiding this comment

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

This commit message should be capitalized (but without period .).

It seems cannot handle vertical lambdas currently. But it's okay to handle this outside this PR.

{ foo
, bar
, baz
, withQux ? false, quz # There are also some mixes.
}:
{
  # ...
}

crates/ide/src/ide/assists/add_to_lambda.rs Outdated Show resolved Hide resolved
crates/ide/src/ide/assists/add_to_lambda.rs Outdated Show resolved Hide resolved
crates/ide/src/ide/assists/add_to_lambda.rs Outdated Show resolved Hide resolved
crates/ide/src/ide/assists/add_to_lambda.rs Outdated Show resolved Hide resolved
crates/ide/src/ide/assists/add_to_lambda.rs Outdated Show resolved Hide resolved
crates/ide/src/ide/assists/add_to_lambda.rs Outdated Show resolved Hide resolved
@oxalica oxalica added C-feature Catagory: feature A-assist Area: assistence/code action labels Dec 4, 2022
@figsoda figsoda changed the title add add_to_lambda assist Add add_to_top_level_lambda_param assist 33b7956 Dec 4, 2022
@figsoda figsoda changed the title Add add_to_top_level_lambda_param assist 33b7956 Add add_to_top_level_lambda_param assist Dec 4, 2022
@figsoda figsoda requested a review from oxalica December 4, 2022 18:43
#[test]
fn simple() {
check("{ }: foo$0", expect!["{ foo }: foo"]);
check("{}: foo$0", expect!["{foo }: foo"]);
Copy link
Owner

Choose a reason for hiding this comment

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

Seems kind of weird. Is this expected?
IMO, braces should always leave spaces inside. From this perspective, this is acceptable.

Copy link
Contributor Author

@figsoda figsoda Dec 4, 2022

Choose a reason for hiding this comment

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

I added the test just to check that the insert-before-space logic doesn't break anything. I don't think it looks good, but I also don't think the extra logic required is worth it for unconventional formatting

@oxalica oxalica merged commit 101387f into oxalica:main Dec 5, 2022
@figsoda figsoda deleted the add-to-lambda branch December 5, 2022 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-assist Area: assistence/code action C-feature Catagory: feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants