Removed redundant space before struct and class declaration#38
Removed redundant space before struct and class declaration#38liamnichols merged 1 commit intoCreateAPI:mainCreateAPI/CreateAPI:mainfrom imjn:remove-redundant-space-before-internal-structimjn/CreateAPI:remove-redundant-space-before-internal-structCopy head branch name to clipboard
Conversation
|
|
||
| func `class`(name: TypeName, contents: [String], protocols: Protocols) -> String { | ||
| let type = options.entities.isMakingClassesFinal ? "final class" : "class" | ||
| let lhs = [options.access, type, name.rawValue].compactMap { $0 } |
There was a problem hiding this comment.
Here we had removed only nil so any empty string had been passing this filter.
There was a problem hiding this comment.
This is where we combine the lhs.
There was a problem hiding this comment.
I also had an idea that I should add the .filter { !$0.isEmpty } to where we combine them like below ↓
private func declaration(lhs: [String], rhs: [String], contents: [String]) -> String {
let lhs = lhs.joined(separator: " ").filter { $0.isEmpty } ⬅️⬅️⬅️
let rhs = rhs.joined(separator: ", ")
return """
\(rhs.isEmpty ? lhs : "\(lhs): \(rhs)") {
\(contents.joined(separator: "\n\n").indented)
}
"""
}With this, we can achieve what we want with less code.
However, I decided to add the filter to func class() and func struct() as they already filter nil and I thought filtering nil and empty string should be in the same place and func class() should not care about that.
What do you think about this...?
There was a problem hiding this comment.
Good questions.. More generally, I think maybe we could improve some of this further since there is also the var access: String computed property at the top of this file (but we can't use it here since it appends whitespace too).
So for now, I think that what you have done is probably fine 👍
There was a problem hiding this comment.
I think ideally, options.access should never be allowed to be an empty string in the first place either. That's something else we might want to look at 😄 If that was the case, the compactMap(_:) would have worked without the need to filter on !isEmpty
There was a problem hiding this comment.
Yes, I think so too. We should unify the way we build a String from multiple Strings at some point. Thanks for your comment!
|
|
||
| func `class`(name: TypeName, contents: [String], protocols: Protocols) -> String { | ||
| let type = options.entities.isMakingClassesFinal ? "final class" : "class" | ||
| let lhs = [options.access, type, name.rawValue].compactMap { $0 } |
There was a problem hiding this comment.
Good questions.. More generally, I think maybe we could improve some of this further since there is also the var access: String computed property at the top of this file (but we can't use it here since it appends whitespace too).
So for now, I think that what you have done is probably fine 👍
closes #32
Background
While we do
lhs.joined(separator: " ")and sometimes have""for the access level, it inserts unnecessary space beforestructandclassdeclaration.What
Remove the redundant space.
How
""too, not onlynil.