Sorry for the late reply... we had a beta release and a new patch level release out this week and I had to fix some bugs in both:-)
On Sun, Jul 3, 2016 at 1:30 PM, Stephen Kelly <steve...@gmail.com> wrote: >> "KDevelop3", > > <off topic> This generator should probably be removed/hidden by now. It has > confused users of more-recent KDevelop versions. Yes, that should be hidden. I'll add that in the next update. >> What do you think? What else should we report here? > > It looks like a good start. The intention of the output is to satisfy needs > that consumers like you have, and I guess it does have what you need. Obviously, or I would not propose it:-) The hope is to get feedback from other potential users. But so far no luck on that front:-/ >> Compared to the bug report mentioned above the fields "multiconfig" and >> "recursive" are missing. I could not figure out how to get that >> information:-/ > > When I made the example in the issue tracker, 'recursive' meant 'you can cd > to a directory and run the CMAKE_MAKE_PROGRAM there'. At the time, it was > possible to do that when using the Makefiles generator, but not the Ninja > generator. > > I think that capability has since been added for the Ninja generator, but I > don't know if it is possible with the Xcode and VS generators. > > Would that be a useful thing to expose here in your view? I think that information would be useful at some point. But it is not needed before a project is opened. >> I would also welcome some code review on the patch. > > 1) For consistency you should change > > GetRegisteredGenerators > > to > > this->GetRegisteredGenerators > > whether that is a preferred style is orthogonal to the fact that it's > consistently used in cmake code. Oh, sorry. I still find that horrible to read, so I only sprinkle this-> over the code shortly before sending it in to review. > 2) CMake has to build with toolchains which do not provide > std::unordered_map. See uses of CMake_HAVE_CXX_UNORDERED_MAP for existing > code which uses either std::unordered_map or std::unordered_map. (Yes there > is room for improvement there, but such improvement is orthogonal to your > branch). Oh, too bad. > 3) Similarly, CMake has to build with compilers which do not support > cxx_range_for or cxx_auto_type. For the cmServer implementation that may or > may not be the case, but within the cmake class, that's the way it is. I'll fix that. > 4) It seems that the > > this->Generators > > member of the cmake class has the names of all generators (without 'extra' > generators). It seems unfortunate to not use that container and instead > parse the names out of the names from the GetRegisteredGenerators call by > splitting on the ' - '. It leads to hard coded magic expressions like > 'separator + 3', so it should be avoided if possible. Yes, that is ugly. > Is there another way of determining the extra generators supported by a > given generator and avoiding parsing a string which we just generated? Can > > const std::vector<std::string>& supportedGlobalGenerators = > extraGenerator->GetSupportedGlobalGenerators(); > > be used somehow? Can you first loop over this->Generators to get the > 'normal' generators, then loop over the extra generators, call that method > to match things up and output the result? Doesn't this require me to instanciate the generators? Won't that be potentially expensive? > Or would it make sense to refactor the container members in the cmake class > themselves to make this information more easily available for this use-case? > > 5) You use the term 'appendix' for the version, but 'suffix' is the more- > commonly used name for that concept. I'll change that. Best Regards, Tobias -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
