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

BartyCrouch.translate generates invalid swiftgenStructured enum if .strings directory contains multiple files #227

Open
lukemmtt opened this issue Jun 22, 2021 · 1 comment

Comments

@lukemmtt
Copy link

lukemmtt commented Jun 22, 2021

Problem

The BartyCrouch.translate command (with transformer = "swiftgenStructured") currently only produces a valid output if there is only one .strings file in the SwiftGen .strings input path specified in swiftgen.yml.

Example

My swiftgen.yml .strings input path is my en.lproj folder, which contains two .strings files:

  • Localizable.strings
  • Main.strings (generated from localizing my storyboard)

My config files are set as follows:

.bartycrouch.toml (only one setting changed from default):
transformer = "swiftgenStructured"

swiftgen.yml:

 strings:
   inputs:
     - /Users/(User)/Developer/AppName/AppName/Resources/en.lproj
   outputs:
     - templateName: structured-swift5
       output: /Users/(User)/Developer/AppName/AppName/Resources/generated/SwiftGen/Strings.swift

To reproduce the issue, I enter the following in one of my view controllers:

BartyCrouch.translate(
            key: "Auth.no-internet-alert.title",
            translations: [.english: "Internet connection required"],
            comment: "Title of prompt shown when an internet connection is not detected upon first launch")

Then I build my project, and the BartyCrouch build script runs, followed by SwiftGen.

Expected Output

BartyCrouch expects that the Strings.swift file generated by SwiftGen for this command will be of the form:

internal enum L10n {
    internal enum Auth {
      internal enum NoInternetAlert {
        /// Internet connection required
        internal static let title = L10n.tr("Localizable", "Auth.no-internet-alert.title")
      }
  }
}

As such, BartyCrouch returns the enum
L10n.Auth.NoInternetAlert.title

Actual Output

When multiple .Strings files are located in the SwiftGen strings input directory, the enum hierarchy generated by SwiftGen in Strings.swift differs from what BartyCrouch expects. To differentiate between the two .strings files' contents, SwiftGen nests each file's strings inside an additional internal enum based on the name of each file.

In this case:

internal enum L10n {
  internal enum Localizable {
    internal enum Auth {
      internal enum NoInternetAlert {
        /// Internet connection required
        internal static let title = L10n.tr("Localizable", "Auth.no-internet-alert.title")
      }
    }
  }
  internal enum Main {
    internal enum _3DVImULc {
      /// Terms of Service
      internal static let text = L10n.tr("Main", "3DV-Im-ULc.text")
    }
  }
}

Since BartyCrouch doesn't expect this, its resulting enum construction L10n.Auth.NoInternetAlert.title is invalid.

A valid enum in this case would be L10n.Localizable.Auth.NoInternetAlert.title

Steps to reproduce

SwiftGen only inserts the filenamed enums in the L10n enum hierarchy under two circumstances:

  1. More than one .strings file is in the input path, or
  2. The forceFileNameEnum SwiftGen parameter is set to true (documented here, source here)
    There is no option within SwiftGen to suppress this nesting behavior if more than one .strings file is present, nor do I think that would be the right approach.

Possible solutions

In order to account for this situation, the buildSwiftgenStructuredExpression method in BartyCrouch's TranslateTransformer class would need to be tweaked.

Option 1 (automated, preferred)

The only foolproof way to address both circumstances outlined above would be to query the swiftgen.yml file directly for its parameters. i.e.:

  1. Find the user's swiftgen.yml file
  2. From that, retrieve the state of the forceFileNameEnum parameter (if present) and the strings input path
  3. Let stringsFileCount = the number of .strings in the input path
  4. Add the following to the buildSwiftgenStructuredExpression method:
      if (stringsFileCount > 1) or (forceFileNameEnum == true) {
        swiftgenKeyComponents.insert("Localizable", at: 1)
      }

Option 2 (manual, user input required)

An alternate solution (less code, but less robust) would be to add a parentEnum parameter to .bartycrouch.toml, which the user can set to "Localizable" or some other custom string, and something like the following to the buildSwiftgenStructuredExpression method:

      if parentEnum != "" {
        swiftgenKeyComponents.insert("parentEnum", at: 1)
      }

Specifications

  • Version: BartyCrouch 4.6.0 & SwiftGen 6.4.0
  • IDE Version: Xcode 12.5.1
@lukemmtt lukemmtt added the bug label Jun 22, 2021
@lukemmtt lukemmtt changed the title BartyCrouch.translate (swiftgenStructured) enum output doesn't align with SwiftGen enums BartyCrouch.translate generates invalid swiftgenStructured enum if .strings directory contains multiple files Jun 22, 2021
@Jeehut
Copy link
Member

Jeehut commented Nov 13, 2021

@lukemmtt I just skimmed through your ideas and haven't invested much time thinking it through. But the immediate thought I had was: "Why don't you simply use a custom SwiftGen template which produces a structure that would be compliant with BartyCrouch?" SwiftGen is based on Stencil and you can easily adjust the template to produce a different structure.

While I don't know why you're using multiple Strings files, also another solution might be – in case you're using one Strings file per module – to simply have multiple BartyCrouch configuration files in different directories and to run the BartyCrouch update command multiple times, once for each directory. I never tested this approach, but it should be possible, you may need to specify the --path parameter to the CLI tool.

But again, I don't believe this to be a "bug", it's simply a new use case BartyCrouch was never designed for. Changing the tag.

@Jeehut Jeehut added enhancement and removed bug labels Nov 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants