| cullmann |
| KTextEditor | |
| RKWard |
This is a javascript indentation script for the statistical language R. This script is for all software using KTextEditor (e.g. Kate), but more specifically for RKWard, which is an IDE for R.
I tested it for corner cases for some time so it should be quite robust, although my ability to find corner cases alone is obviously limited by a lack of diversity in coding styles.
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
I think this is very cool. But there is one thing we require for new indenters: unit tests.
There is a subfolder autotest in the KTextEditor git module. Could you have a look into this? Without such automated tests, we will likely introduce regressions in future updates of the indenter. If you need help, please let us know :-)
| src/script/data/indentation/r.js | ||
|---|---|---|
| 4 ↗ | (On Diff #33259) | Would you also agree to use MIT license? MIT ist preferred for Javascript and XML highlighting files. |
| 6 ↗ | (On Diff #33259) | I believe 5.0 is good enough. There were no breaking changes between 5.0 and 5.1. |
Hi,
OK, I think I see how it works by looking at the folder. Though I'm not a professional dev, so the notion of unit test is a bit blur for me (I understand it's about testing a very precise case with a template output to compare to?). I can have a go at it, but it might take some time...
No problem for changing the licence and the version requirement. I just copied what I found on the Python script, as far as I remember...
Regarding the unit tests: Take a look at e.g. https://cgit.kde.org/ktexteditor.git/tree/autotests/input/indent/cmake/enter1 . You start with an initial text "origin", then emulate some input "input.js", an specify the expected state at the end "expected".
Again, I have no experience with indentation scripts, so no good idea what they should look like. A nitpick / idea is inlined.
| src/script/data/indentation/r.js | ||
|---|---|---|
| 174 ↗ | (On Diff #33259) | I wonder if this cannot be simplified (but not sure I understand the complete logic). Pseudo-code: var brackets = new Array();
for ([iterating backwards, skipping strings and comments]) {
var current_char = [...];
var closing_type = closings.indexOf(current_char);
if (closing_type >= 0) {
brackets.push(current_char);
} else {
var opening_type = openings.indexOf(current_char);
if (opening_type >= 0 && brackets.pop() != current_char) {
[mismatch: return]
}
[...]
}
} |
@devillemereuil If I remember correctly, you do not have to run all tests. It is enough to start the indenter test (look into the bin folder) and add as parameter you indent test folder (relative, e.g. cstyle). And, what Thomas says is correct. Typically a test case only contains very few lines to test a specific case. I think once you get the first one running, it will be easy for you to add more.
I will have a go at it. It doesn't seem very complicated indeed, but I just don't have much time right now for this.
@tfry I'm not sure what your pseudo-code is intended to replace? The parts using countClosing? FYI, these snippets of code were copied from the Python script. I learned JS on the go here, which is why I'm using very little idiosyncrasies from the language (apart from what I saw on the other scripts, mainly Python and Cstyle, with a few things picked up on Stack Overflow, I've got to admit...), so it's quite possible the script is not as elegant as it could be.
@devillemereuil This was meant to replace calcMismatchIndent(), or at least the upper part of it. But don't let me distract you, here, this is non-essential. Just focus on the unit tests for now.
Given it seems nobody has time to do tests, I will merge this.
We can still remove it again but just letting rot this nice contribution in the phabricator won't help any R user.
Just to say: this didn't fall off my radar. I'm just incredibly busy nowadays and it never quite make it to the top of my todo list... But I think about doing it regularly, which is a good sign it'll happen!
EDIT: Also, thanks for merging!
@devillemereuil That is good to hear. If you can, feel free to paste a MIT-licensed snippet of R-code with a function, a loop, a if-condition etc. Maybe we find the time to turn this into a unit test. The code does not need to run, it just needs to be syntactically correct.
I might get to that around September (no promise). If people are willing to help, my not-unit-at-all test code for indentation can be found here:
https://github.com/devillemereuil/rindent/blob/master/example.R
Automatic indent should not impact the code (this is how I test my commits), meaning it is already indented as the script is willing to indent.
Say that I have the files for a unit test (origin, input.js and expected). How can I check the code for the unit test? Is there a way to simply execute the javascript code and verify it does what I expect? It would be a shame to have the unit test fail because of the code test itself.
The easiest way is just to put the test in the autotests input like for other highlighters and then run the indent test, that will pick this up and tell you what happens.
So, basically, I just write a bunch of tests, commit them here and we'll debug them if needed from here?
You don't need to commit them.
You just can add them locally test by test and after each one run the indent test locally.
That's the thing: I don't know how to run the test. If it requires dealing with cmake and stuff, that's take me a while to figure out. Is there any simple documentation about it?
Ok,
here the basic idea:
in autotests/src/identtest.cpp and .h
add some
void testR_data(); void testR();
add some
void IndentTest::testR_data()
{
getTestData("R");}
void IndentTest::testR()
{
runTest(ExpectedFailures());
}
Then, after make, you should be able to run
./bin/kateindenttest
and it should start to use the test data in the R dir
I added these lines
https://commits.kde.org/ktexteditor/784302a921b8e1fafd6d32cf172552eaea1775bf
If you do make, you can later in the build folder run:
./bin/kateindenttest
ATM it will tell at the end:
SKIP : IndentTest::testR() /home/cullmann/local/kde/src/frameworks/ktexteditor/autotests/input/indent/R/ does not exist
Loc: [/home/cullmann/local/kde/src/frameworks/ktexteditor/autotests/src/script_test_base.cpp(89)]
You might need a .kateconfig in the R folder like in e.g. the cstyle folder which tells "use R highlighting and R indenter"
OK. So, that'll require compile ktexteditor from source. I should be able to manage that (I guess?). I'll report here if it gives me too many headaches. I tried today, but gave up because of an issue with ECM (?) version. I got:
Could not find a configuration file for package "ECM" that is compatible with requested version "5.50.0". The following configuration files were considered but not accepted: /usr/share/ECM/cmake/ECMConfig.cmake, version: 5.49.0
I'll find whether I can circumvent/fix this issue somehow.
Hi,
the message means you need to update the extra-cmake-modules.
How did you compile the stuff? If you use kdesrc-build, I would just let that run once more.
I... git cloned ktexteditor and ran cmake .? Is there a better way? Sorry, I'm a real newb in this as you can probably tell! I'm not a dev by training.
I normally follow the steps on
https://community.kde.org/Guidelines_and_HOWTOs/Build_from_source
and just do kdesrc-build ktexteditor after the initial setup.