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

Using CoreParse to parse NSS file #198

Merged
merged 2 commits into from
Jan 19, 2014
Merged

Conversation

phatmann
Copy link
Contributor

Fixes #33.

This pull request integrates CoreParse to parse the NSS file. CoreParse uses BNF syntax and thus makes extending the NSS format fairly easy. The next step is to provide @media queries per #3, which is my real goal here.

I just submitted a pull request to CoreParse that adds a podspec file. Once that gets approved I will place the podspec file in the CocoaPods Specs repo and I will update the Podfile in the NUI Demo accordingly.

tombenner added a commit that referenced this pull request Jan 19, 2014
Using CoreParse to parse NSS file
@tombenner tombenner merged commit d81e391 into tombenner:master Jan 19, 2014
@tombenner
Copy link
Owner

Excellent, thanks! This is crucial to NUI's continued improvement; very much appreciated.

@tombenner
Copy link
Owner

Yes, this is an unfortunate situation; I'm torn between the options. What are your thoughts on what the best approach would be?

@siuying
Copy link

siuying commented Jan 23, 2014

Subspec will work, but if two library both dependent on CoreParse then we will get nasty duplicate symbol error.

@siuying
Copy link

siuying commented Jan 23, 2014

Include a static library and public headers of CoreParse may work. Still can have duplicate symbol error.

Seriously I think the only sane way to do it is maintain the CoreParse podspec and keep it updated.

@phatmann phatmann deleted the core-parse branch January 23, 2014 18:03
@phatmann
Copy link
Contributor Author

@tombenner, @siuying is desperately trying to convince the author of CoreParse to change his mind and provide a podspec. Let's wait a day or two for the response. Thanks @siuying!

If the author of CoreParse decides against providing a podspec, I suggest that you fork CoreParse, rename it (e.g. NUIParse), and add my podspec to the fork. As @siuying says, that is the only "sane" approach. We will need to keep our fork in sync with CoreParse, but this is pretty easy to do with an upstream remote.

@siuying
Copy link

siuying commented Jan 24, 2014

Yeah I have a project also dependent on CoreParse and now still thinking how to properly release next version.

The problem is not author not providing a podspec, but reject the idea of someone else provide a podspec.

Forking the project and rename it make me feel bad by not attributing this great project and author. Also others might now start to dependent on your fork and you waste time on maintain it (rather than maintain the podspec).

@siuying
Copy link

siuying commented Jan 24, 2014

I might just add a podspec for CoreParse anyway, this is not making the author happy, but I guess is still better than keeping the same thing privately or fork the entire project.

@phatmann
Copy link
Contributor Author

A few thoughts:

  • Even if we use a fork, we would give the author the 100% proper credit he is due. I would explain that the fork exists solely so we can have a pod, and also explain that we are not taking pull requests on the fork (unless the changes affect the podspec).
  • Maintaining a fork is not hard. CoreParse is not that active, and merging in the occasional updates is simple if we don't make any other changes to the code.

@tombenner
Copy link
Owner

Thanks, @siuying and @phatmann. Let's see if there are any new developments on this in the next day or so, and if not, perhaps the route that @phatmann outlines would be our best path forward.

@siuying
Copy link

siuying commented Jan 27, 2014

I uploaded a spec to CocoaPods.

@phatmann
Copy link
Contributor Author

@siuying, excellent, it works perfectly. Thanks!

@tombenner, when you get a chance, you can change the CoreParse entry in NUiDemo's Podfile so it does not point to my CoreParse fork.

@tombenner
Copy link
Owner

Great--I've pushed the new NUI spec to Cocoapods/Specs and have updated NUIDemo's Podfile. Thanks again, @siuying and @phatmann!

@leberwurstsaft
Copy link

Somehow I can't get the :head of CoreParse when I also add the NUI pod to my Podfile.
With NUI installing CoreParse as a dependency it always ends up installing version 1.1 of it.
Without NUI and simply pod 'CoreParse', :head I will get the HEAD based on 1.1 version.

Has anybody got a clue why this is and how it can be solved? The 1.1 version of CoreParse has bugs and is quite old.

@siuying
Copy link

siuying commented Jan 29, 2014

I tested and it's OK. I include CoreParse first then NUI later in my Podfile like this:

  pod 'CoreParse', :head
  pod 'NUI'

@leberwurstsaft
Copy link

Thanks for testing. I had

pod 'CoreParse', :head
pod 'NUI', '~> 0.5.1'

and tried without 0.5.1 now. It worked. Went back to 0.5.1 and it still works. I blame Cocoapods 😉

@siuying
Copy link

siuying commented Jan 29, 2014

Yes, I believe it is related to this CocoaPods/CocoaPods#540 where it mentioned CocoaPods dependency resolution is not very advanced like that of bundler and it can have problem resolving such case.

@leberwurstsaft
Copy link

I see, well I'm glad it's working ... for now.

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.

Improve the NSS Parser
4 participants