Added the possibility to use $VARIABLE in environment settings.
After some discussion it was concluded, that we won't support defining a variable with other user defined variable. Only supported scenario is defining a variable with a system variable.
Details
Tested with the added unit test and in practice.
Diff Detail
- Repository
- R33 KDevPlatform
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Looks good already, just a few issues.
Thanks
util/environmentgrouplist.cpp | ||
---|---|---|
210 | Coding style:
| |
util/environmentgrouplist.h | ||
26 | Forward decl is enough | |
util/tests/test_environment.cpp | ||
2 | Add license header | |
8 | Newline after this line | |
9 | Please use data-driven tests here. This is a perfect fit. | |
11 | I'd suggest to build your own QProcessEnvironment (cf. QProcessEnvironment::insert, etc.), and pre-fill with values for PATH/HOME. Then testing code gets easier, and you can use plain strings. | |
util/tests/test_environment.h | ||
2 | Add license header |
util/environmentgrouplist.h | ||
---|---|---|
26 | What do you mean? Removing this line brakes the build with error "QProcessEnvironment not defined" |
util/environmentgrouplist.h | ||
---|---|---|
26 | class QProcessEnvironment; here | |
27 | Remove space | |
142 | Nitpick: No space after identifier. Newline before and after. | |
util/tests/test_environment.cpp | ||
35 | Please avoid overlong lines, hard to read in the diff viewer already. ... << ProcEnv{ {key, value}, {key, value} } | |
util/tests/test_environment.h | ||
1 | Nitpick: Use a nicer formatting for the license :) /* * DESC * * Copyright NAME * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU Library General Public License as * published by the Free Software Foundation; either version 2 of the * License, or (at your option) any later version. * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU General Public License for more details. * * You should have received a copy of the GNU General Public * License along with this program; if not, write to the * Free Software Foundation, Inc., * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ | |
22 | Newline before |
Looks good to me!
Will test a bit and then push. But I think your GCI task is done. Thanks a lot for your patience and patch, great work!
On hold for now.
This doesn't work as intended for the major use-case: Native app jobs. They're not even based on OutputExecuteJob. And I have no idea why; it would make a lot of sense as far as I understand...
util/tests/test_environment.cpp | ||
---|---|---|
77 ↗ | (On Diff #1823) | "env" and "environment" isn't the best naming. |
util/tests/test_environment.cpp | ||
---|---|---|
77 ↗ | (On Diff #1823) | Will fix. Thanks I accidentally pushed a WIP patch anway (thus all the qDebug calls...). arc fail. |