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 option to change tab width #15

Merged
merged 2 commits into from
Mar 30, 2022
Merged

add option to change tab width #15

merged 2 commits into from
Mar 30, 2022

Conversation

rruiter87
Copy link
Contributor

Added it for me, maybe you'd like it as well?

Copy link
Contributor

@klauer klauer 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 the PR @rruiter87 - I think it's nice to have this be customizable.

Looking back, two spaces probably would have been a good choice starting out. Since we have 4 spaces for all of our projects at the moment, though, reformatting every xml file might be a bit much.

Edit: on second thought, my morning brain had me confused with our Python source settings. 2 has been the default. Sorry about that!

Roping in @ZLLentz to get his input before jumping ahead.

@klauer klauer requested a review from ZLLentz March 29, 2022 16:00
@@ -34,6 +36,7 @@ def main(args=None):
if args is None:
parser = argparse.ArgumentParser()
parser.add_argument('filenames', nargs='*')
parser.add_argument('--tab-width', type=int, default=TAB_WIDTH)
Copy link
Member

Choose a reason for hiding this comment

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

Hi, thanks for the contribution!

However, I'm confused, is there a commit missing here? This argparse argument you added isn't used below in the call to fix_file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! There is a missing commit indeed. I noticed the argument was missing when I was testing it locally, but forgot to push it 😅

Copy link
Member

Choose a reason for hiding this comment

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

I figured as much, it happens all the time! I'm going to go ahead and merge this.
Thanks again for the pull request!

@ZLLentz ZLLentz merged commit 0b41c4f into pcdshub:master Mar 30, 2022
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.

3 participants