[SourceKit] Move Code Formatting Logic to libIDE #1686
Conversation
regnerjr
changed the title
|
@swift-ci Please test |
|
@akyrtzi Please review. |
|
Hi John, Thanks for working on this! I think we should squash/rebase some of the commits together to make the changes more independent. We aim for every commit to be able to build/test successfully on its own. I don't think we want them all squashed into one commit, but if we can make them more independent/logical it will be easier to review them.
These should be rebased into the preceding commits. Basically the Headers should show up in t he same commit that the new file is added and the rebase change should be squashed into wherever the issue came from.
Similarly, these look like they should be part of preceding commits. Every commit should independent and be able to build and test successfully on its own.
More commits that should be squashed somewhere. |
|
@swift-ci Please test |
| size_t getExpandedIndentForLine(unsigned LineIndex, CodeFormatOptions Options, | ||
| StringRef Text); | ||
|
|
||
| class SwiftEditorLineRange { |
There was a problem hiding this comment.
How about "SourceLineRange", or even just "LineRange"? SwiftEditor doesn't really add anything here.
|
Hi @benlangmuir & @akyrtzi I cleaned up the history and corrected the errors that @benlangmuir pointed out yesterday. Let me know if you see anything that needs more work. Thanks! |
| @@ -1491,7 +1491,7 @@ class CodeFormatter { | |||
| } | |||
| Builder.append(Line); | |||
| Consumer.recordFormattedText(Builder.str().str()); | |||
| return SwiftEditorLineRange(LineIndex, 1); | |||
| return std::pair<SwiftEditorLineRange, StringRef> (SwiftEditorLineRange(LineIndex, 1), Text); | |||
|
This is looking awesome! I've made a few more suggestions on the individual commits here. Thanks for your hard work! |
Move FormatContext & FormatWalker into new file. Move CodeFormatOptions out of SwiftEditorDocument. Move functions getTrimmedLineOffset & getLineOffset out of SwiftEditor Document. Rename Offset of Line functions Declaring that the get the offset of some line index is more clear.
This helps de-couple us from the Swift Editor Document.
Moved getTrimmedTextForLine & getExpandedIndentForLine This help reduce the coupling to SwiftEditor Document and allows us to format any text, whether or not is currently in an editor doc.
CodeFormatter no longer depends on EditorDocument
This makes it easire to format all text, nomatter its source.
Also cleaned up whitespace
Move all the rest of the code formatters and helper classes out of the interface.
|
Hey @benlangmuir! Thanks so much for you help. I am now a rebase, rewriting history master. Thanks for the great suggestions. I think I have now addressed all your comments, and ensured that each individual commit, builds and passes all tests. Let me know if there is anything more you'd like to see here. Thanks again. |
|
@swift-ci Please test |
[SourceKit] Move Code Formatting Logic to libIDE
[SourceKit] Move Code Formatting from SourceKit -> libIDE
Moved CodeFormatter, FormatContext, FormatWalker and associated classes, structures, functions to libIDE.
I have run the tests locally (OS X)
utils/build-script -tNo new tests have been added since no features were added or removed.
This PR makes progress on - SR-146 but does not fulfill all of its scope.
Related bug number: (SR-146)
Ping: @akyrtzi @benlangmuir
Before merging this pull request to apple/swift repository:
Triggering Swift CI
The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:
Smoke Testing
Validation Testing
Note: Only members of the Apple organization can trigger swift-ci.