Skip to content

Add outlining spans for Import declarations#23894

Merged
mhegazy merged 11 commits into
masterfrom
importOutlining
May 7, 2018
Merged

Add outlining spans for Import declarations#23894
mhegazy merged 11 commits into
masterfrom
importOutlining

Conversation

@mhegazy
Copy link
Copy Markdown
Contributor

@mhegazy mhegazy commented May 4, 2018

This PR adds new outlining spans for import declarations. This adds:

  • New kind to OutliningSpanKind for import
  • Create span for consecutive import declarations
    image

@mhegazy mhegazy requested a review from uniqueiniquity May 4, 2018 18:39
@sheetalkamat
Copy link
Copy Markdown
Member

Should the consecutive imports be named sequentialStatements or something similar so we can extend that in future for other statements if need to.
What about exports?

case SyntaxKind.ArrayLiteralExpression:
return spanForObjectOrArrayLiteral(n, SyntaxKind.OpenBracketToken);
}
}
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.

Stray indentation

Copy link
Copy Markdown
Member

@DanielRosenwasser DanielRosenwasser left a comment

Choose a reason for hiding this comment

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

We must have other code like this somewhere...

visitImportNode(statements[current] as AnyImportSyntax, sourceFile, cancellationToken, out);
current++;
}
const lastImport = current < n ? statements[current - 1] : statements[n - 1];
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.

I think this is always just current - 1 since as soon as current === n... well, yeah

@mhegazy
Copy link
Copy Markdown
Contributor Author

mhegazy commented May 4, 2018

Should the consecutive imports be named sequentialStatements or something similar so we can extend that in future for other statements if need to.

sure i can do this. i will probably call them imports. just to be specific. i think VSCode wanted to hide all imports automatically to save screen realstate in the file.

What about exports?

I originally started by adding these, but then it looked weird with only export declarations and not other declarations with export modifiers on them. so i walked that back.

Copy link
Copy Markdown
Contributor

@uniqueiniquity uniqueiniquity left a comment

Choose a reason for hiding this comment

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

Looks good to me given Daniel's suggested changes.

@mhegazy
Copy link
Copy Markdown
Contributor Author

mhegazy commented May 5, 2018

Updated the PR to change the outlining span kind name to imports instead of import, remove the outlining span for named import binding, and the stray indentation change.

@mhegazy
Copy link
Copy Markdown
Contributor Author

mhegazy commented May 5, 2018

@DanielRosenwasser other comments?

@mhegazy mhegazy merged commit e39e6fc into master May 7, 2018
@mhegazy mhegazy deleted the importOutlining branch May 7, 2018 19:08
@microsoft microsoft locked and limited conversation to collaborators Jul 31, 2018
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.

4 participants