Re: [PATCH] userdiff: add support for Swift

Johannes Sixt <j6t@kdbg.org> writes:

You are right. The pattern only matches attributes that are inline with the declaration. An attribute on its own line is not matched, and does not need to be, because the declaration line below it matches on its own. I have reworded this.

[...]

Fixed in a coming v2: swift-init, swift-failable-init and swift-generic-subscript now contain "RIGHT" only once, on the declaration line.

Noted, and thanks for the follow-up on this one. I did check it anyway out of curiosity: with that shape, a line that is only modifiers and never reaches a real declaration keyword, such as

	public var counter = 0

would still match, because it merges modifiers and declaration keywords into one interchangeable run. The current pattern requires a real keyword at the end, so that line correctly gets no header. I will keep the current form for now, and can revisit if the backtracking turns out to matter in practice.

Right -- that is why I kept them as three separate patterns, so the digit ranges stay correct (binary [01], octal [0-7]); merging would mis-tokenize "0b1_abc".

No -- Swift requires a leading digit, so ".5" is a syntax error (one must write "0.5"). Tokenizing it as "." and "5" is therefore fine, and it does not occur in valid Swift.

Done in a coming v2, thanks; I had not realized PATTERNS appends "|[^[:space:]]". It is a nice simplification, and it only touches the word regex, not the funcname pattern.

Since neither of us speaks Swift, for your ease of judgement I have also put together some coverage numbers, which the coming v2 cover note will include:

 - Grammar: I went through every declaration form listed in the "Summary
   of the Grammar" in Swift's own language reference (func, init incl.
   failable/generic, deinit, subscript incl. generic, class, struct,
   enum, protocol, extension, actor, operator methods, stacked
   modifiers, attributes with and without arguments, "where" clauses,
   multi-line signatures -- 26 forms total) and wrote a case for each.
   All 26 get the correct header.

 - Real-world code: I ran the driver over the last 200 commits touching
   *.swift in seven different Swift projects -- Alamofire,
   apple/swift-argument-parser, vapor, Kingfisher, RxSwift, SnapKit, and
   pointfreeco/swift-composable-architecture -- and checked every hunk
   header by hand. Out of 20454 hunks, 15310 got a header, and 15296 of
   those (99.9%) named a real declaration. None of the empty-header
   hunks turned out to be a real miss (they were things like file
   comment blocks, imports, or Package.swift, which have nothing to
   attach a header to).

These numbers are unaffected by the changes in this reply: the funcname pattern is identical in v1 and v2 (only the word regex and the test files changed), and both measurements are of hunk headers, which come from the funcname pattern alone. So the coverage above still holds for v2.

Besides the fixes above, v2 will also carry the reworded attribute description and the changelog explaining what changed since v1, so the full picture is in one place when you look at it.

Thanks for the careful review. Shlok