Review Request 122193: ECMGenerateHeaders: look for lower-case.h if lowercase.h does not exist

[Daniel Vrátil]

> On Jan. 30, 2015, 12:15 a.m., Friedrich W. H. Kossebau wrote:
> > Aha, am not the only one with a patch about "Various projects use various naming conventions, so supporting only lowercase.h headers in ECMGenerateHeaders is a bit limiting" :)
> > Just that my case are CamelCase.h headers, see https://git.reviewboard.kde.org/r/122317/
> > No idea yet how these two patches could be aligned, but ideally they would. Will think about some more next week.
> 
> Daniel Vrátil wrote:
>     I think we might extend your patch so that in addition to LOWERCASE and CAMELCASE it also supports something like LOWERCASE_SEPARATOR and CAMELCASE_SEPARATOR, and addiong ORIGINAL_SEPARATOR option to specify whether dash or underscore is used? This would probably cover most cases that make sense.
>     
>     I can look into extending your patch and send you a mail, alternatively we could just merge yours (if Alex is OK with it), and will rebase this patch on top of your new changes.
> 
> Alex Merry wrote:
>     I think explicit arguments like in Friedrich's patch are the way to go. If you're willing to sort out a patch combining both features, I don't mind where it ends up, though.
> 
> Friedrich W. H. Kossebau wrote:
>     Of course I would not mind simply already committing my patch, that way my issues are already scratched ;) and I know the next release will cover it, independing if further discussions will hold up this patch here. Just, as I learned by this patch, unit tests are possible also with ecm, so I should add one.
>     
>     WRT to explicit arguments, being strict about only filenames with separators or not, I wonder if separators are used consistently with filenames. One real-word example from KDE repos: include/KF5/konq_historyentry.h for class KonqHistoryEntry. So some people use separators only randomly, so not to separate any words, but just a few. So it might make sense to extend this patch to only optionally expect separators (hm, perhaps by using `file(GLOB)` to get a list of all candidates, and if no=1 take that, otherwise complain), unless we are okay with enforcing people to rename their original header files to match exactly the pattern expected.

I'll just rebase my patch on yours once it's pushed, I'm not in hurry to get my changes in :)

I think there's a limit to how "clever" this utility should be - handling all possible and impossible naming conventions is probably not worth the added complexity - it will at least force the devs to fix their naming conventions ;-)


- Daniel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122193/#review75015
-----------------------------------------------------------


On Jan. 21, 2015, 10:25 p.m., Daniel Vrátil wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122193/
> -----------------------------------------------------------
> 
> (Updated Jan. 21, 2015, 10:25 p.m.)
> 
> 
> Review request for Build System.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> -------
> 
> Various projects use various naming conventions, so supporting only lowercase.h headers in ECMGenerateHeaders is a bit limiting :) This patch adds a fallback which will try to look for lower-case.h header file in case lowercase.h does not exist.
> 
> 
> Diffs
> -----
> 
>   modules/ECMGenerateHeaders.cmake bac5086 
>   tests/ECMGenerateHeadersTest/run_test.cmake.config 0a2425f 
> 
> Diff: https://git.reviewboard.kde.org/r/122193/diff/
> 
> 
> Testing
> -------
> 
> Added a unit-test which seems to pass.
> 
> 
> Thanks,
> 
> Daniel Vrátil
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-buildsystem/attachments/20150206/10aa60a3/attachment.html>