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

Chose formatting settings that matched current formatting as much as possible #24

Merged
merged 1 commit into from
Nov 29, 2021
Merged

Chose formatting settings that matched current formatting as much as possible #24

merged 1 commit into from
Nov 29, 2021

Conversation

jhoek
Copy link
Contributor

@jhoek jhoek commented Nov 26, 2021

Note that this pull request does not include any "functional changes".

@jhoek
Copy link
Contributor Author

jhoek commented Nov 26, 2021

Resolves #23.

@rvanbekkum
Copy link
Owner

Looks good. 😊 Do you know if it is possible to have else and elseif statements on a new line?

@jhoek
Copy link
Contributor Author

jhoek commented Nov 27, 2021

I guess the easiest way to control these things is to set powershell.codeFormatting.preset. For examples of the different options, please take a look at PoshCode/PowerShellPracticeAndStyle#81. Please let me which style you prefer, and I'll update my pull request.

@rvanbekkum
Copy link
Owner

Then I think my preference would be to use OTBS, but with the else on a new line. 😅
Luckily I am not the only one:
PoshCode/PowerShellPracticeAndStyle#81 (comment)

@jhoek
Copy link
Contributor Author

jhoek commented Nov 28, 2021

I'm not sure if that's something that can be done, and, if I'm entirely honest, I fail to see the benefit. How about if we opt for the OTBS for now, and consider making an exception for else later?

@rvanbekkum
Copy link
Owner

Yes, sounds good to me. 😉

@jhoek
Copy link
Contributor Author

jhoek commented Nov 29, 2021

In that case, I guess you could approve this PR? :-)

@rvanbekkum
Copy link
Owner

Surely. 😊

@rvanbekkum rvanbekkum merged commit aecdcf1 into rvanbekkum:develop Nov 29, 2021
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