Config to only add j2objc_shared method to Podfile

[saruye]

@brunobowden
I started working on issue #561. This is not finished. I just opened this PR already to get some feedback if I am on the right track.

My idea was: Add an extra property that can disable updating the targets. Default will be as it is. If the property is set, it will disable all checks for targets and only add/update the j2objc method to the Podfile. Is that the intention?

My questions:
naming: since you renamed the ticket I guess you prefer the naming to be something about "only to add the method" instead of "disabling updating of the target"? in the task I used updateTargets now as a boolean since that seemed more straight forward. Is that ok?
So far the method is always added before the target instructions. Where should it go if this cannot be found? Is it possible to just add it at the beginning of the file? Maybe before the 'source ...' ?

[brunobowden]

1. Rename so that it's more clearly associated with the xcodeTargetsIos config:

xcodeTargetsManualConfig

2. Add the javadoc to it indicating what it does:

/**
 * Allows manual config of the Podfile (default is false).
 *
 * When set to true, this allows manual configuring of the Podfile targets.
 * This is necessary when your Podfile is too complex to be automatically
 * updated. When used, you must also set xcodeTargets{Ios|Osx|Watchos)
 * to empty.
 */

3. move this after the xcodeTargetsWatchos setting, so it's the last one listed.

[brunobowden]

Hi @saruye, thanks for your work on this. I think that generally this is on the right track. I came across this when looking at some complex Podfiles, so I thought that eventually people would need it.

To answer your questions:

My questions:
naming: since you renamed the ticket I guess you prefer the naming to be something about "only to add the method" instead of "disabling updating of the target"?

• I wasn't consciously thinking about this. I generally think of naming a setting being the most important for someone using the plugin, then secondly important for someone reading the code as a developer. In this case, I think it's helpful to associate it with the xcodeTargetsIos-like settings... then also explain when it's most likely needed (e.g. the verifyTargets exception).

in the task I used updateTargets now as a boolean since that seemed more straight forward. Is that ok?

• Boolean is fine and makes sense. For a developer reading the code, I find tremendous value to have consistent naming throughout the stack. The same name from the config to the internal variable is very helpful. That's lost when you change names and meanings.

So far the method is always added before the target instructions. Where should it go if this cannot be found?

• Fair point. Part of the value for this is the ability to remove and update the methods when they change over time. By placing the code before the target, there's an ability to easily wipe and replace it. If there's no target then that becomes harder.

Two thoughts:

1. Probably need a START and END of the custom shared methods. So if this is found, then it's updated in place, regardless of where it's moved to - not sure if Ruby requires you to declare methods before they're used.

# J2ObjC Gradle Plugin - DO NOT MODIFY from here to similar line below"
# This block may be moved as a complete unit around the Podfile
...
# J2ObjC Gradle Plugin - DO NOT MODIFY from here to similar line above"

2. When trying to place the shared code, use the existing if any, then try to do it before the first targets, if both of those aren't possible, then do it after the end of the first comment section.

Is it possible to just add it at the beginning of the file? Maybe before the 'source ...' ?

• I think you're on the right track. I don't know you mean by source..., this is what I see in my Podfile:

# Uncomment this line to define a global platform for your project
# platform :ios, '6.0' asdf 

use_frameworks!

So I think that line 3 being the first line that isn't a comment could work. As above though, if the code has been moved elsewhere in the file, then it should be updated in place.

[superarne]

I tried to implement all your feedback. I didn't get to implement the actual updating logic though. My idea is to replace the regexStripLines with somethingLike regexReplaceLines and then update the insert logic only to run if nothing was replaced and to insert it at the beginning if no targetStartRegex is found. Try to get to it sometime this week.

[brunobowden]

We've generally avoided the style of extra indentation for multiple lines. So lines 408 and 409 should start at the same place as line 407.

This applies several times below as well.

[brunobowden]

@superarne, @saruye - are you the same user? It's just a little more confusing when you're using different accounts.

This is definitely coming along. Definitely make sure that all the unit tests pass. When it's ready, ping me again all I'll go do a final review.

[saruye]

hi. yeah that was me. I was logged in with a wrong account. sry :/
I implemented the logic now for adding the generated method to the podfile. I split it into two parts. One that looks where to put it and the other one that does the insert trying to make sure it has empty lines before and after. I did update one or two existing tests where I thought the new behavior with empty lines is nicer. I added a test with a Podfile which looks similar to ours. This and all old tests pass.
Is there by any chance a codestyle file for intellij that I could import?
Could you have a look at me code and my assumptions regarding where to insert the new lines and if you would like more/less/other tests. Don't bother with the style problems for now. I will fix those once you are fine with the general logic. Also if there is something too ungroovy let me know.

[brunobowden]

This should be a variation of the regexInsertLines method. Just extend regexInsertLines, to add a boolean replace parameter. If true, it should skip copying the old lines. It avoid having duplication of similar code.

[brunobowden]

@saruye - I took elements of your code and then reworked it for PR #582. With that in mind, I'm closing this PR but I'd still appreciate your input on the other one. Thank you for helping out here.