From 56769a7f60a55d4664b251654dab8c0f11d792f3 Mon Sep 17 00:00:00 2001 From: Andrea Laren Date: Mon, 29 Dec 2014 23:09:57 -0500 Subject: [PATCH 1/2] Fixes handling of localized project files Made changes so synx understands Xcode's weird subfolder structure for localized files and folders (PBXVariantGroup), added a file with multiple localizations to the dummy project, and updated the tests. --- lib/synx/xcodeproj_ext.rb | 1 - .../project/object/abstract_object.rb | 35 ++++++++++------ .../project/object/pbx_file_reference.rb | 4 +- .../xcodeproj_ext/project/object/pbx_group.rb | 42 +++++++++++-------- .../project/object/pbx_variant_group.rb | 35 ---------------- spec/dummy/dummy.xcodeproj/project.pbxproj | 15 +++---- spec/dummy/dummy/en.lproj/Localizable.strings | 2 + .../en.lproj/de.lproj/Localizable.strings | 9 ++++ spec/synx/expected_file_structure.yml | 2 + spec/synx/project_spec.rb | 2 +- 10 files changed, 67 insertions(+), 80 deletions(-) delete mode 100644 lib/synx/xcodeproj_ext/project/object/pbx_variant_group.rb create mode 100644 spec/dummy/dummy/en.lproj/de.lproj/Localizable.strings diff --git a/lib/synx/xcodeproj_ext.rb b/lib/synx/xcodeproj_ext.rb index 939a332..e6d48bd 100644 --- a/lib/synx/xcodeproj_ext.rb +++ b/lib/synx/xcodeproj_ext.rb @@ -4,4 +4,3 @@ require "synx/xcodeproj_ext/project/object/pbx_file_reference" require "synx/xcodeproj_ext/project/object/pbx_group" require "synx/xcodeproj_ext/project/object/abstract_object" require "synx/xcodeproj_ext/project/object/abstract_target" -require "synx/xcodeproj_ext/project/object/pbx_variant_group" diff --git a/lib/synx/xcodeproj_ext/project/object/abstract_object.rb b/lib/synx/xcodeproj_ext/project/object/abstract_object.rb index 8e7cdf4..9402578 100644 --- a/lib/synx/xcodeproj_ext/project/object/abstract_object.rb +++ b/lib/synx/xcodeproj_ext/project/object/abstract_object.rb @@ -8,27 +8,36 @@ module Xcodeproj end def referring_groups - referrers.select { |ref| ref.instance_of?(Xcodeproj::Project::Object::PBXGroup) } + referrers.select { |ref| ref.is_a?(Xcodeproj::Project::Object::PBXGroup) } end def work_pathname - # hierarchy path has a leading '/' that will break path concatenation - @work_pathname ||= project.work_root_pathname + hierarchy_path[1..-1] + # Intuitively, we want the work pathname to correspond 1-1 with the + # view in the project hierarchy. Xcode's collapsed display of + # identically-named localized files causes some complications, leading + # to the special cases here. + if self.equal?(project.main_group) + @work_pathname ||= project.work_root_pathname + elsif parent.is_a?(Xcodeproj::Project::Object::PBXVariantGroup) + # Localized object, naming is handled differently. + @work_pathname ||= parent.work_pathname + "#{display_name}.lproj" + parent.display_name + elsif is_a?(Xcodeproj::Project::Object::PBXVariantGroup) + # Localized container, has no path of its own. + @work_pathname ||= parent.work_pathname + else + @work_pathname ||= parent.work_pathname + display_name + end end def ensure_internal_consistency(group) @removed_from_groups = [] - if referring_groups.count > 1 - # Files should only have one referring group -- this is an internal consistency issue if there is more than 1. - # Just remove all referring groups but the one we're syncing with - - referring_groups.each do |rg| - unless rg == group - rg.remove_reference(self) unless rg == group - @removed_from_groups << rg.hierarchy_path - end + # Objects should only have one referring group -- this is an internal consistency issue if there is more than 1. + # Just remove all referring groups but the one we're passed + referring_groups.each do |rg| + unless rg == group + rg.remove_reference(self) + @removed_from_groups << rg.hierarchy_path end - end end diff --git a/lib/synx/xcodeproj_ext/project/object/pbx_file_reference.rb b/lib/synx/xcodeproj_ext/project/object/pbx_file_reference.rb index f36ba24..a95a6f4 100644 --- a/lib/synx/xcodeproj_ext/project/object/pbx_file_reference.rb +++ b/lib/synx/xcodeproj_ext/project/object/pbx_file_reference.rb @@ -6,10 +6,10 @@ module Xcodeproj def sync(group) if should_sync? if should_move? - FileUtils.mv(real_path.to_s, group.work_pathname.to_s) + FileUtils.mv(real_path.to_s, work_pathname.to_s) # TODO: move out to abstract_object self.source_tree = "" - self.path = real_path.basename.to_s + self.path = work_pathname.relative_path_from(parent.work_pathname).to_s else # Don't move this file around -- it's not even inside the structure. Just fix the relative reference self.path = real_path.relative_path_from((project.work_pathname_to_pathname(group.work_pathname))).to_s diff --git a/lib/synx/xcodeproj_ext/project/object/pbx_group.rb b/lib/synx/xcodeproj_ext/project/object/pbx_group.rb index 7c4e6c4..aabd06f 100644 --- a/lib/synx/xcodeproj_ext/project/object/pbx_group.rb +++ b/lib/synx/xcodeproj_ext/project/object/pbx_group.rb @@ -4,7 +4,7 @@ module Xcodeproj class PBXGroup def sync(group) - ensure_internal_consistency(group) + ensure_internal_consistency(group) # Make sure we don't belong to any other groups if excluded_from_sync? Synx::Tabber.puts "#{basename}/ (excluded)".yellow else @@ -12,9 +12,17 @@ module Xcodeproj Synx::Tabber.increase squash_duplicate_file_references - work_pathname.mkpath - files.each { |pbx_file| pbx_file.sync(self) } - all_groups.each { |group| group.sync(self) } + # Child directories may not exist yet (and may be different for + # each file) if this is a localized group, so we do the mkpath call + # inside the loops. + files.each do |pbx_file| + pbx_file.work_pathname.dirname.mkpath + pbx_file.sync(self) + end + all_groups.each do |group| + group.work_pathname.dirname.mkpath + group.sync(self) + end sync_path Synx::Tabber.decrease @@ -31,7 +39,11 @@ module Xcodeproj else Synx::Tabber.puts "#{basename}/".green Synx::Tabber.increase - Dir[real_path.to_s + "/{*,.*}"].reject { |e| %W(. ..).include?(Pathname(e).basename.to_s) }.each do |entry| + Dir[real_path.to_s + "/{*,.*}"] + .reject { |e| %W(. ..).include?(Pathname(e).basename.to_s) } + .each do |entry| + # Is this right? entry should be an absolute path here, so it should + # overwrite real_path entirely in this sum, which seems counterintuitive. entry_pathname = real_path + entry unless project.has_object_for_pathname?(entry_pathname) handle_unused_entry(entry_pathname) @@ -43,32 +55,24 @@ module Xcodeproj end def sync_path - self.path = basename + self.path = work_pathname.relative_path_from(parent.work_pathname).to_s self.source_tree = "" end private :sync_path def all_groups - groups | version_groups | variant_groups + children.select { |child| child.is_a?(Xcodeproj::Project::Object::PBXGroup)} end - def variant_groups - children.select { |child| child.instance_of?(Xcodeproj::Project::Object::PBXVariantGroup) } - end - private :variant_groups - def handle_unused_entry(entry_pathname) entries_to_ignore = %W(.DS_Store) unless entries_to_ignore.include?(entry_pathname.basename.to_s) if entry_pathname.directory? - work_entry_pathname = project.pathname_to_work_pathname(entry_pathname) - # The directory may have already been created for one of two reasons - # 1. It was created as a piece of another path, ie, /this/middle/directory.mkdir got called. - # 2. OS X has case insensitive folder names, so has_object_for_pathname? may have failed to notice it had the folder. - work_entry_pathname.mkdir unless work_entry_pathname.exist? # recurse Synx::Tabber.puts entry_pathname.basename.to_s.green Synx::Tabber.increase + # Don't create the directory manually: if it has children, it will + # be created then, and if it doesn't, we don't want it. entry_pathname.children.each { |child| handle_unused_entry(child) } Synx::Tabber.decrease elsif entry_pathname.file? @@ -86,7 +90,9 @@ module Xcodeproj Synx::Tabber.puts "#{file_pathname.basename} (removed source/image file that is not referenced by the Xcode project)".red return elsif !project.prune || !is_file_to_prune - FileUtils.mv(file_pathname.realpath, project.pathname_to_work_pathname(file_pathname.parent).realpath) + destination = project.pathname_to_work_pathname(file_pathname.parent.realpath) + destination.mkpath + FileUtils.mv(file_pathname.realpath, destination) if is_file_to_prune Synx::Tabber.puts "#{file_pathname.basename} (source/image file that is not referenced by the Xcode project)".yellow else diff --git a/lib/synx/xcodeproj_ext/project/object/pbx_variant_group.rb b/lib/synx/xcodeproj_ext/project/object/pbx_variant_group.rb deleted file mode 100644 index 3a7c69c..0000000 --- a/lib/synx/xcodeproj_ext/project/object/pbx_variant_group.rb +++ /dev/null @@ -1,35 +0,0 @@ -module Xcodeproj - class Project - module Object - class PBXVariantGroup - - # Need to retain *.lproj files on the system - def sync(group) - ensure_internal_consistency(group) - - file = files.first - if lproj_as_group? - FileUtils.mv(file.real_path, work_pathname) - Synx::Tabber.puts file.real_path.basename.to_s.green - else - parent_folder_path = children.first.real_path.parent - work_destination_pathname = parent.work_pathname - - if parent_folder_path.exist? - FileUtils.mv(parent_folder_path, work_destination_pathname.realpath) - end - Synx::Tabber.puts (parent_folder_path.basename.to_s + "/").green - Synx::Tabber.increase - Synx::Tabber.puts file.real_path.basename.to_s.green - Synx::Tabber.decrease - end - end - - def lproj_as_group? - parent.basename =~ /.+\.lproj$/ - end - - end - end - end -end diff --git a/spec/dummy/dummy.xcodeproj/project.pbxproj b/spec/dummy/dummy.xcodeproj/project.pbxproj index c6fa148..c9a98e3 100644 --- a/spec/dummy/dummy.xcodeproj/project.pbxproj +++ b/spec/dummy/dummy.xcodeproj/project.pbxproj @@ -62,6 +62,7 @@ /* End PBXCopyFilesBuildPhase section */ /* Begin PBXFileReference section */ + 637E22771A5235CE00A08D6A /* de */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = de; path = de.lproj/Localizable.strings; sourceTree = ""; }; 8C2DEB7D191D3F5C003A1F44 /* ManyFiles.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ManyFiles.h; sourceTree = ""; }; 8C2DEB7E191D3F5C003A1F44 /* ManyFiles.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ManyFiles.m; sourceTree = ""; }; 8C2DEB80191D3F68003A1F44 /* Wow.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = Wow.h; sourceTree = ""; }; @@ -207,7 +208,7 @@ 8C848C4E190DB9B300E9727B /* Supporting Files */ = { isa = PBXGroup; children = ( - 8CD2ABF019558D4100341C58 /* en.lproj */, + 8CD2ABF119558D7800341C58 /* Localizable.strings */, 8C848C4F190DB9B300E9727B /* dummy-Prefix.pch */, ); name = "Supporting Files"; @@ -227,7 +228,6 @@ isa = PBXGroup; children = ( 8C848C63190DB9B300E9727B /* dummyTests-Info.plist */, - 8C848C64190DB9B300E9727B /* InfoPlist.strings */, 8CE2DA1D19220F7B00D06F5E /* dummyTests-prefix.pch */, ); name = "Supporting Files"; @@ -249,14 +249,6 @@ name = Resources; sourceTree = ""; }; - 8CD2ABF019558D4100341C58 /* en.lproj */ = { - isa = PBXGroup; - children = ( - 8CD2ABF119558D7800341C58 /* Localizable.strings */, - ); - path = en.lproj; - sourceTree = ""; - }; 8CDA046219374F15004435A1 /* FolderWithGroupNotLinked */ = { isa = PBXGroup; children = ( @@ -318,6 +310,7 @@ hasScannedForEncodings = 0; knownRegions = ( en, + de, ); mainGroup = 8C848C3F190DB9B300E9727B; productRefGroup = 8C848C49190DB9B300E9727B /* Products */; @@ -389,8 +382,10 @@ isa = PBXVariantGroup; children = ( 8CD2ABF219558D7800341C58 /* en */, + 637E22771A5235CE00A08D6A /* de */, ); name = Localizable.strings; + path = en.lproj; sourceTree = ""; }; /* End PBXVariantGroup section */ diff --git a/spec/dummy/dummy/en.lproj/Localizable.strings b/spec/dummy/dummy/en.lproj/Localizable.strings index eda5965..d9c2519 100644 --- a/spec/dummy/dummy/en.lproj/Localizable.strings +++ b/spec/dummy/dummy/en.lproj/Localizable.strings @@ -5,3 +5,5 @@ Created by Mark Larsen on 6/21/14. Copyright (c) 2014 marklarr. All rights reserved. */ + +"Greeting" = "Hello"; \ No newline at end of file diff --git a/spec/dummy/dummy/en.lproj/de.lproj/Localizable.strings b/spec/dummy/dummy/en.lproj/de.lproj/Localizable.strings new file mode 100644 index 0000000..0d865e2 --- /dev/null +++ b/spec/dummy/dummy/en.lproj/de.lproj/Localizable.strings @@ -0,0 +1,9 @@ +/* + Localizable.strings + dummy + + Created by Mark Larsen on 6/21/14. + Copyright (c) 2014 marklarr. All rights reserved. +*/ + +"Greeting" = "Guten Tag"; \ No newline at end of file diff --git a/spec/synx/expected_file_structure.yml b/spec/synx/expected_file_structure.yml index 09b528d..f1ccb98 100644 --- a/spec/synx/expected_file_structure.yml +++ b/spec/synx/expected_file_structure.yml @@ -30,6 +30,8 @@ dummy: dummy-Prefix.pch: en.lproj: Localizable.strings: + de.lproj: + Localizable.strings: dummy.h: dummy.m: FileNotInXcodeProj.h: diff --git a/spec/synx/project_spec.rb b/spec/synx/project_spec.rb index 3ec3bd4..0a1cbad 100644 --- a/spec/synx/project_spec.rb +++ b/spec/synx/project_spec.rb @@ -131,7 +131,7 @@ describe Synx::Project do expected_file_structure_with_removals = expected_file_structure expected_file_structure_with_removals["dummy"].except!("image-not-in-xcodeproj.png") expected_file_structure_with_removals["dummy"].except!("FileNotInXcodeProj.h") - expected_file_structure_with_removals["dummy"]["AlreadySynced"]["FolderNotInXcodeProj"].except!("AnotherFileNotInXcodeProj.h") + expected_file_structure_with_removals["dummy"]["AlreadySynced"].except!("FolderNotInXcodeProj") verify_file_structure(Pathname(DUMMY_SYNX_TEST_PROJECT_PATH).parent, expected_file_structure_with_removals) end From e9dfa3cd8e032ced89439739e5a5214f789a5833 Mon Sep 17 00:00:00 2001 From: Andrea Laren Date: Wed, 31 Dec 2014 20:16:33 -0500 Subject: [PATCH 2/2] Minor path cleanup --- lib/synx/xcodeproj_ext/project/object/pbx_group.rb | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/lib/synx/xcodeproj_ext/project/object/pbx_group.rb b/lib/synx/xcodeproj_ext/project/object/pbx_group.rb index aabd06f..1eadaaa 100644 --- a/lib/synx/xcodeproj_ext/project/object/pbx_group.rb +++ b/lib/synx/xcodeproj_ext/project/object/pbx_group.rb @@ -36,15 +36,10 @@ module Xcodeproj def move_entries_not_in_xcodeproj if excluded_from_sync? Synx::Tabber.puts "#{basename}/ (excluded)".yellow - else + elsif real_path.exist? Synx::Tabber.puts "#{basename}/".green Synx::Tabber.increase - Dir[real_path.to_s + "/{*,.*}"] - .reject { |e| %W(. ..).include?(Pathname(e).basename.to_s) } - .each do |entry| - # Is this right? entry should be an absolute path here, so it should - # overwrite real_path entirely in this sum, which seems counterintuitive. - entry_pathname = real_path + entry + real_path.children.each do |entry_pathname| unless project.has_object_for_pathname?(entry_pathname) handle_unused_entry(entry_pathname) end