nicklockwood / SwiftFormat

A command-line tool and Xcode Extension for formatting Swift code
MIT License
7.92k stars 639 forks source link

Support a consistent date format in the file header #1507

Closed hampustagerud closed 1 year ago

hampustagerud commented 1 year ago

Hello again πŸ‘‹ I came across another issue when formatting the header, which was that systems with a different locale would format all files differently (as stated in the documentation). However, I wanted to avoid different formatting on different systems to avoid unnecessary diffs. I implemented an option for formatting the date consistently, either using a unicode format or a predefined to make it a bit easier πŸ™‚

But then I thought: different locales could probably mean different timezones as well so an option to decide a consistent timezone was implemented to supplement the date format option.

I noticed when writing tests that using the created timestamp from the operating system is relative to when the file was created locally, meaning different systems would have different timestamps. So I made the GitHelper able to also fetch the created timestamp for a file since it is already fetching other "creation" information about the file. It has a fallback to the current behaviour.

When I could get consistent creation dates it became apparent that it only tracked until the file was put in it's current place, since I didn't use --follow when calling git to keep tracking it across renames. So there is now a placeholder {created} and {created.follow} to allow choosing which date is of interest. I tested this in the SwiftFormat repo and it didn't work for one file (LinuxMain.swift) which has been deleted and then put back later, this is ignored and the git helper will just use the latest date it was created (even in follow mode).

Finally, there was a bug in my last PR that has been fixed so it no longer crashes. I have added tests to make sure it's working properly, including the shell access. I have also rewritten the git helpers to reduce the amount of calls needed to the shell (from 560 to 190 when running in this repo) with memoization.

This PR became larger than I had expected but I thought it was just another option but it snowballed a bit. Let me know your thoughts on it and if you want me to try to split it up a bit. πŸ™‚

codecov[bot] commented 1 year ago

Codecov Report

Merging #1507 (4428a4d) into develop (0d0819d) will increase coverage by 0.14%. The diff coverage is 90.69%.

:exclamation: Current head 4428a4d differs from pull request most recent head 8e0ce11. Consider uploading reports for the commit 8e0ce11 to get more accurate results

@@             Coverage Diff             @@
##           develop    #1507      +/-   ##
===========================================
+ Coverage    94.96%   95.10%   +0.14%     
===========================================
  Files           20       20              
  Lines        21033    21351     +318     
===========================================
+ Hits         19973    20305     +332     
+ Misses        1060     1046      -14     
Files Changed Coverage Ξ”
Sources/Arguments.swift 95.24% <ΓΈ> (ΓΈ)
Sources/Options.swift 92.23% <84.21%> (-3.77%) :arrow_down:
Sources/GitHelpers.swift 82.85% <85.86%> (+82.85%) :arrow_up:
Sources/ShellHelpers.swift 93.33% <90.90%> (+93.33%) :arrow_up:
Sources/CommandLine.swift 69.65% <100.00%> (-0.35%) :arrow_down:
Sources/Examples.swift 99.93% <100.00%> (+<0.01%) :arrow_up:
Sources/FormattingHelpers.swift 97.69% <100.00%> (+0.01%) :arrow_up:
Sources/OptionDescriptor.swift 99.14% <100.00%> (+0.12%) :arrow_up:
Sources/Rules.swift 98.54% <100.00%> (+<0.01%) :arrow_up:
Sources/SwiftFormat.swift 90.07% <100.00%> (+0.19%) :arrow_up:
nicklockwood commented 1 year ago

@hampustagerud this is great, thanks! I might make a few tweaks before release, but I'll land as-is for now to prevent conflicts.

hisaac commented 5 months ago

I see this PR has been merged to develop for a while. Any chance it will be released soon @nicklockwood? I'd love to use the new tokens added here.

nicklockwood commented 5 months ago

@hampustagerud I'm just working on adding this to the next release, and I had a couple of follow-up questions:

1) under what circumstances would you not want to use follow? Wouldn't it make sense just to make this the standard behavior for all tokens?

2) do you find you ever actually want to include the creator email in a file header? I've never seen this done

hampustagerud commented 5 months ago
  1. It would make sense to me to always use follow when available, the git history is usually considered the source of truth so I think it could be made the standard behaviour πŸ‘ It was tacked on as an opt-in behaviour to keep backwards compatibility
  2. I personally haven't used it myself (yet), it was more one of those "it's easily available and might be useful" kind of things. In some projects (not necessarily Swift projects though) there's a convention to use My Name <my.name@email.com> in the header. If so, this could be useful I guess πŸ™‚