Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for file header template #2568

Merged
merged 25 commits into from Mar 9, 2021
Merged

Add support for file header template #2568

merged 25 commits into from Mar 9, 2021

Conversation

olejnjak
Copy link
Collaborator

@olejnjak olejnjak commented Feb 27, 2021

Short description πŸ“

As working in team various file header templates might apply. This is done by using IDETemplateMacros.plist inside xcodeproj. This plist might be shared so located under xcshareddata and also user-specific. This PR currently addresses shared data.

Currently this PR is a draft to discuss the implementation, tests and docs will be added afterwards.

As it might be desired to add also user-specific macros, I implemented new ProjectMapper that is responsible for created the above mentioned plist. If later the Config is extended with user-specific macros then the IDETemplateMacrosProjectMapper can be easily updated to create more plists.

Checklist βœ…

  • The code architecture and patterns are consistent with the rest of the codebase.
  • The changes have been tested following the documented guidelines.
  • The CHANGELOG.md has been updated to reflect the changes. In case of a breaking change, it's been flagged as such.
  • In case the PR introduces changes that affect users, the documentation has been updated.

@olejnjak olejnjak marked this pull request as draft February 27, 2021 01:46
@olejnjak olejnjak changed the title Draft: Add support for file header template Add support for file header template Feb 27, 2021
Copy link
Collaborator

@kwridan kwridan left a comment

Choose a reason for hiding this comment

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

Nice work @olejnjak thanks for submitting this!

I like the overall approach and +1 for using a mapper for this as it fits quite nicely!

Some thoughts:

  • There's talk of deprecating Config.generationOptions :/ Deprecating global project attributes in favour of helpersΒ #2429 in favour of per Manifest configuration. Co-incidentally the same topic was brought up here Per-Project Resource AccessorsΒ #2564 too regarding how to define this per project.

  • If I'm not mistaken, header templates can be applied at the workspace level too which automatically get applied to all projects with it

    • This would mean the template header is only ever created once per workspace rather than for every individual project, we can convert the mapper to a WorkspaceMapper and apply the same logic you have already applied here such that the plist is placed in <WorkspaceName>.xcworkspace/xcshareddata/IDETemplateMacros.plist
    • If we proceed with discussions above regarding deprecating config, perhaps having this at the workspace level could be a bit easier to adopt globally rather than per-project (thought I still see the value of the config defined approach so not sure how to proceed with this one :/ )
  • Worth adding a fixture to show-case this in the various forms :)

@olejnjak olejnjak force-pushed the file_header branch 3 times, most recently from 7ea7b3d to 15e34a4 Compare March 7, 2021 19:05
@olejnjak
Copy link
Collaborator Author

olejnjak commented Mar 7, 2021

Should be ready right now with following changes:

  • IDETemplateMacrosMapper now supports not just project mapping, but also workspace mapping as it should basically do the same job regarding projects and workspaces (add a plist into its shared data)
  • when creating IDETemplateMacros with template string that should be used, various "normalizations" are performed to easily achieve desired state
    • remove extra comment slashes if appropriate as they are added automatically by Xcode
    • add extra leading space if template doesn't start with comment slashes and doesn't start with whitespace
    • remove extra traling space as it is automatically added by Xcode

@olejnjak olejnjak marked this pull request as ready for review March 7, 2021 19:23
Copy link
Collaborator

@kwridan kwridan left a comment

Choose a reason for hiding this comment

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

Awesome work @olejnjak - thanks for all the updates, it's looking great!

@kwridan kwridan merged commit ff10fa1 into main Mar 9, 2021
@kwridan kwridan deleted the file_header branch March 9, 2021 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants