D15002: Allow to install syntax files instead of having them in a resource
cullmann closed this revision. cullmann added a comment. If somebody has an fix for the CMake thingy, please open a new diffusion for that. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D15002 To: cullmann, vkrause, dhaumann Cc: kfunk, dhaumann, kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, bruns, demsking, cullmann, sars
D15002: Allow to install syntax files instead of having them in a resource
cullmann reopened this revision. cullmann added a comment. This revision is now accepted and ready to land. If you have ideas how, patches are welcome. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D15002 To: cullmann, vkrause, dhaumann Cc: kfunk, dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, ngraham, bruns, demsking, cullmann, sars
D15002: Allow to install syntax files instead of having them in a resource
kfunk added inline comments.
INLINE COMMENTS
> CMakeLists.txt:3
> macro(generate_php_syntax_definition targetFile srcFile)
> -add_custom_command(
> -OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/${targetFile}
> -COMMAND ${PERL_EXECUTABLE}
> ${CMAKE_CURRENT_SOURCE_DIR}/generators/generate-php.pl <
> ${CMAKE_CURRENT_SOURCE_DIR}/syntax/${srcFile} >
> ${CMAKE_CURRENT_BINARY_DIR}/${targetFile}
> -DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/generators/generate-php.pl
> ${CMAKE_CURRENT_SOURCE_DIR}/syntax/${srcFile}
> -)
> +execute_process(COMMAND ${CMAKE_COMMAND} -E make_directory
> ${CMAKE_CURRENT_BINARY_DIR}/syntax)
> +execute_process(COMMAND ${PERL_EXECUTABLE}
> ${CMAKE_CURRENT_SOURCE_DIR}/generators/generate-php.pl
Note: The Ninja generator doesn't like this:
CMake Warning (dev):
Policy CMP0058 is not set: Ninja requires custom command byproducts to be
explicit. Run "cmake --help-policy CMP0058" for policy details. Use the
cmake_policy command to set the policy and suppress this warning.
This project specifies custom command DEPENDS on files in the build tree
that are not specified as the OUTPUT or BYPRODUCTS of any
add_custom_command or add_custom_target:
data/syntax/css-php.xml
data/syntax/html-php.xml
data/syntax/javascript-php.xml
For compatibility with versions of CMake that did not have the BYPRODUCTS
option, CMake is generating phony rules for such files to convince 'ninja'
to build.
Project authors should add the missing BYPRODUCTS or OUTPUT options to the
custom commands that produce these files.
This warning is for project developers. Use -Wno-dev to suppress it.
There must be a way to implement this feature and keep using
`add_custom_command`...? Using `execute_process()` is usually a bad sign (tm).
Note that these commands will be executed each CMake run(!)
REPOSITORY
R216 Syntax Highlighting
REVISION DETAIL
https://phabricator.kde.org/D15002
To: cullmann, vkrause, dhaumann
Cc: kfunk, dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, ngraham,
bruns, demsking, cullmann, sars
D15002: Allow to install syntax files instead of having them in a resource
This revision was automatically updated to reflect the committed changes. Closed by commit R216:59ddf56f98d9: Allow to install syntax files instead of having them in a resource (authored by cullmann). REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15002?vs=40242=40247 REVISION DETAIL https://phabricator.kde.org/D15002 AFFECTED FILES CMakeLists.txt autotests/foldingtest.cpp autotests/highlighter_benchmark.cpp autotests/htmlhighlighter_test.cpp autotests/repository_benchmark.cpp autotests/syntaxrepository_test.cpp autotests/test-config.h.in autotests/testhighlighter.cpp autotests/theme_test.cpp data/CMakeLists.txt src/lib/CMakeLists.txt src/lib/repository.cpp To: cullmann, vkrause, dhaumann Cc: dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars
D15002: Allow to install syntax files instead of having them in a resource
dhaumann accepted this revision. This revision is now accepted and ready to land. REVISION DETAIL https://phabricator.kde.org/D15002 To: cullmann, vkrause, dhaumann Cc: dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars
D15002: Allow to install syntax files instead of having them in a resource
cullmann requested review of this revision. cullmann added a comment. Updated patch to allow make test to work. REVISION DETAIL https://phabricator.kde.org/D15002 To: cullmann, vkrause Cc: dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars
D15002: Allow to install syntax files instead of having them in a resource
cullmann updated this revision to Diff 40242. cullmann added a comment. Update that allows make test to succeed, even before one does make install. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15002?vs=40232=40242 REVISION DETAIL https://phabricator.kde.org/D15002 AFFECTED FILES CMakeLists.txt autotests/foldingtest.cpp autotests/highlighter_benchmark.cpp autotests/htmlhighlighter_test.cpp autotests/repository_benchmark.cpp autotests/syntaxrepository_test.cpp autotests/test-config.h.in autotests/testhighlighter.cpp autotests/theme_test.cpp data/CMakeLists.txt src/lib/CMakeLists.txt src/lib/repository.cpp To: cullmann, vkrause Cc: dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars
D15002: Allow to install syntax files instead of having them in a resource
vkrause accepted this revision. vkrause added a comment. This revision is now accepted and ready to land. makes sense REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D15002 To: cullmann, vkrause Cc: dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars
D15002: Allow to install syntax files instead of having them in a resource
dhaumann added a comment. BTW, slightly related as idea: I think it would be nice to add a function Definition Downloader::setDownloadLocation() or similar that allows to have a custom download location. This would match nicely with Repository::addCustomSearchPath(). REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D15002 To: cullmann, vkrause Cc: dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars
D15002: Allow to install syntax files instead of having them in a resource
dhaumann added a comment. I am fine with this change. I want a review from Volker, through . REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D15002 To: cullmann, vkrause Cc: dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars
D15002: Allow to install syntax files instead of having them in a resource
cullmann created this revision. cullmann added a reviewer: vkrause. Herald added projects: Kate, Frameworks. Herald added subscribers: kde-frameworks-devel, kwrite-devel. cullmann requested review of this revision. REVISION SUMMARY Given the license of some files is dubious, this allows MIT loving people to install them as files and not bundling them. TEST PLAN make works make test fails ATM, but that needs more love if that idea is accepted at all. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D15002 AFFECTED FILES CMakeLists.txt data/CMakeLists.txt src/lib/CMakeLists.txt src/lib/repository.cpp To: cullmann, vkrause Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars, dhaumann
