erisu opened a new pull request, #167:
URL: https://github.com/apache/cordova-node-xcode/pull/167

   It appears that the changes from #14 were not added to the source grammar 
file.
   If the parser were ever rebuilt, those changes would have been reverted.
   
   Additionally, when I added those changes to the grammar source file, the 
parser test for `section-split` failed.
   
   Based on commit 
https://github.com/apache/cordova-node-xcode/commit/d3bf486ac2c137eb29e0eafc49a89b67ee6f0535,
 it appears that properties of multiple sections with the same name should be 
able to be merged together and that Xcode is able to handle this correctly. 
While this might not be common, some tools may produce a project with split 
sections.
   
   The changes from #14 replaced the `merge_obj` logic with `Object.assign`, 
resulting in a shallow merge. As a result, previously parsed entries from an 
earlier section with the same name would have been overwritten.
   
   The original commit's `merge_obj` method was a recursive deep-merge, 
triggered for every property, and may have resulted in the "Maximum call stack 
size exceeded" error that PR #14 was attempting to resolve.
   
   To support split sections, I believe we should shallow-merge the top-level 
properties of each section rather than the entire object. Because this only 
merges one level deeper and is still not a recursive deep merge, I believe it 
could continue to avoid the "Maximum call stack size exceeded" error that PR 
#14 was addressing.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to