Skip to content

Expose indentation suppressor from SmartIndenter#4757

Merged
mhegazy merged 12 commits intomicrosoft:masterfrom
saschanaz:indentSuppressor
Dec 9, 2015
Merged

Expose indentation suppressor from SmartIndenter#4757
mhegazy merged 12 commits intomicrosoft:masterfrom
saschanaz:indentSuppressor

Conversation

@saschanaz
Copy link
Copy Markdown
Contributor

Separated from #4609.

shouldIndentChildNode function already have indentation suppressing feature, but currently auto-formatter disables it to get delta value and implements another suppressor in computeIndentation function.

This PR split the suppressor from shouldIndentChildNode and expose it as a separate function so that formatter can use it without implementing its own one.

The function will help suppress indentation of:

Note: shouldInheritParentIndentation and its internal nodeWillIndentChild receives TextRangeWithKind to check more conditions other than syntax kind.

Comment thread src/services/formatting/formatting.ts Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use undefined, not null

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@saschanaz saschanaz changed the title Add shouldInheritParentIndentation function to SmartIndenter Expose indentation suppressor from SmartIndenter Sep 14, 2015
Comment thread src/services/formatting/formatting.ts Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

undefined not null

@RyanCavanaugh
Copy link
Copy Markdown
Member

Ping @vladima

Comment thread src/services/formatting/formatting.ts Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

to me condition SI.shouldInheritParentIndentation() ? 0 : delta look more clear and it is doing the same thing.

  • if delta is not-null-or-zero then body of getDelta boils down to SI.shouldInheritParentIndentation() ? 0 : delta
  • if delta zero then getDelta evaluates to just delta which will mean the same

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You're right, it's doing the same thing. I was to pass the SI check when delta value is already zero but I agree that it looks less clear. Do you want me to fix this?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yes, please

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done!

Comment thread src/services/formatting/formatting.ts Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

remove parens around child

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this should be marked as /@internal/

@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented Dec 9, 2015

sorry for the delay. Can you add the @internal annotation in your next PR.

mhegazy added a commit that referenced this pull request Dec 9, 2015
Expose indentation suppressor from SmartIndenter
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants