We have a couple of hundred cases where a source file includes an include file like this: #include "../foo/bar.hxx" or #include <../foo/bar.hxx> That is generally considered ugly, I think. In most cases, the file being included should be moved to an appropriate "inc" directory in the module in question, and then be included with a simple: #include "bar.xx" because if an include file is used by source files in multiple directories, clearly it isn't strictly local and should be in some "inc" directory. One special case is the gtk2/gtk3 mess in vcl, which probably should be left as is.
Tor Lillqvist committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=5eb7692ab845a879d265b2e4df48cb7e4536da33 tdf#95416: Get rid of #include "../foo/bar.hxx" style includes It will be available in 5.1.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
(The above is meant to be an example, done for just one module.) (And when we say "module", we mean subdirectory, nothing fancier.)
Status -> NEW.
Migrating Whiteboard tags to Keywords: (easyHack, difficultyBeginner, skillCpp, topicCleanup) [NinjaEdit]
burcinakalin committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=6536150492b8f59af3385afbab7792082f1b45dd tdf#95416 Get rid of #include "../foo/bar.hxx" style includes It will be available in 5.2.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
burcinakalin committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=6a00a34d374bc1780b1448d10750e4129214e294 tdf#95416 Get rid of #include "../foo/bar.hxx" style includes It will be available in 5.2.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
burcinakalin committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=078d5e8abf51a576dd8532d479fb39bfcb8a4e97 tdf#95416 Get rid of #include "../foo/bar.hxx" style includes. It will be available in 5.2.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
JanI is default CC for Easy Hacks (Add Jan; remove LibreOffice Dev List from CC) [NinjaEdit]
Burcin Akalin committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=60fbe5099bb174039a9d9b397477607c9be50e78 tdf#95416 Get rid of #include "../foo/bar.hxx" style includes It will be available in 5.2.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Controlled, still open
Please be aware, that this easyhack is considered an important but large scale cosmetic change as described in https://wiki.documentfoundation.org/Development/LargeScaleChanges It was in decided by the ESC to close this kind of easyhacks, and send them directly as mail, to new contributors. https://lists.freedesktop.org/archives/libreoffice/2016-August/074920.html Please do not submit patches with many files !! This particular easyhack is kept open as an exception to the rule
Jochen Nitschke committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=290b25af222124051cc611061de9caf8f1efe966 tdf#95416 remove ../ style include It will be available in 5.3.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Jochen Nitschke committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=c922161117dcaf33d6c61b0c5ffe860231e32e3c tdf#95416 remove ../ style include It will be available in 5.3.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Dilek Uzulmez committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=403bb51074848cd3f78c13eef14ce306e4f86501 tdf#95416 Get rid of #include ../foo/bar.hxx style includes It will be available in 5.3.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Arnold Dumas committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=2b3cf45bb557d95aa67b5bc4eb1ba549e7638d6b tdf#95416: Get rid of #include ../foo/bar.hxx style includes It will be available in 5.3.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
There are still 218 occurrences of this, by my count, using git grep -E '#include (<|")\.\./' I'll take a look at this and maybe write a script that replaces all of these occurrences (except the gtk* ones mentioned)
By all means do write a script to help you, but don't blindly just run a script on all instances and assume the result compiles on all platforms. Do it in pieces, and wait for confirmation from Jenkins that it actually compiles on all platforms.
(In reply to Tor Lillqvist from comment #17) > By all means do write a script to help you, but don't blindly just run a > script on all instances and assume the result compiles on all platforms. Do > it in pieces, and wait for confirmation from Jenkins that it actually > compiles on all platforms. Yes of course.
Adam Kasztenny committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=13cba3505f3af25b640e9d3fa8e24ccdf1378c68 tdf#95416 Fix an include for xmlsecurity It will be available in 5.4.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Adam Kasztenny committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=ef65c40518c6f4671f9528edade9947092e10ac0 tdf#95416 Fix includes for xmloff It will be available in 5.4.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Adam Kasztenny committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=69d11029d6c72078381f33918574c5a0e043e217 tdf#95416 Fix an include for sfx2 It will be available in 5.4.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Huzaifa Iftikhar committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=9abac96518125dddd29ef1e2b4b84429f8ca88a5 tdf#95416 Fix an include for desktop directory It will be available in 5.4.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Huzaifa Iftikhar committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=fe18f5d51fcbdf1f331440265ba1b8866d03e459 tdf#95416 Fix an include for 2 files in unopkg It will be available in 5.4.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
tamsil1amani3 committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=7a07610588893a2061eaafead36cdd8eabb2b52b tdf#95416 Fix include file It will be available in 5.4.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Gaurav Dhingra committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=f6ff601b820749898a045e115cc0dc460e996104 tdf#95416 Get rid of #include ../foo/bar.hxx style includes It will be available in 5.4.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
abdulwd committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=a0f6374670f67646e802aef45d927a8746b2ae12 tdf#95416 Two include files moved to sc/inc It will be available in 5.4.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
other than the case of gtk2/gtk3 in vcl, I have checked that such cases still exists in nearly 4 - 5 other folders, so, can I assign this task to myself. I guess I will be able to do it, without any errors. What I understood is, we just need to make a local copy of the required include file to the folder, which actually uses it ( by ../ ).
No *copying* of files should be done. Just look at one of the examples that you see of what has already been done here.
so, do we need to move the include file, from ../../ to the inc folder of the parent directory.
Jeevan committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=a5a6694128728c48c1a8482450a21ad49025c40c tdf#95416 Removed ../ in filter/source/svg/test files It will be available in 5.4.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Jeevan committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=befafd4f8d58f81ac9cb0e4ef8dfbda9373787f7 tdf#95416 remove ../ in #include headers of sfx2 files It will be available in 5.4.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
ARGH! Can we please be VERY SENSIBLE about this? It leads to blindly moving headers into a module's global inc/ directory, making them available to each and every source file of that module effectively destroying the separation of view,core,filter even further. Examples of such bad moves are https://cgit.freedesktop.org/libreoffice/core/commit/?id=a0f6374670f67646e802aef45d927a8746b2ae12 moving Calc ViewData around and the just pending https://gerrit.libreoffice.org/34827 that even wanted to move ScDocShell which would be an invitation to use it about ~everywhere. I rather prefer having some sporadic #include "../../ui/inc/xxx.hxx" if really necessary than letting run this loose. Specifically UI view related classes are not to be offered for general use even if now and then they may be needed by filter code. I also consider deciding how to solve things by no means an EasyHack in all cases unless we don't care about model separation anymore. Or am I just on the wrong track and everything is fine?
But isn't it *using* stuff from a wrong include file that indicates something is wrong, not where the include file is located? Anyway, I'm fine with resolving this as NOTABUG; Eike is right that no easy hacker will understand the subtleties involved. Not even I do. Should we instead then have a difficultyInteresting easyHack (a contradiction in terms...) to find these includes and fix the code to not need stuff from such a "wrong" include file at all?
Jeevan - so sorry you hit something that turned out to be controversial ! =) at least finding that out was helpful for the project; so good work. Any my 2 cents on headers; personally I -really- like headers to be well scoped - Calc is quite good here, but even so -so- many modules like to use ../inc/ for headers that would be IMHO -much- better placed right next to the source file they refer to, and which have no usage outside that scope which I find rather irritating personally ;-) But - each to their own ... =)
(In reply to Tor Lillqvist from comment #33) > But isn't it *using* stuff from a wrong include file that indicates > something is wrong, not where the include file is located? Yes it is, though maybe inevitable in some filter code cases, but moving the file to a higher hierarchy level and adapting the include effectively hides that and invites for further abuse all over the place. > Should we instead then have a difficultyInteresting easyHack (a > contradiction in terms...) to find these includes and fix the code to not > need stuff from such a "wrong" include file at all? We can try.. but having taken a look at the mentioned ScDocShell case it's definitely not just a five minute task [1] ... and no quick idea how it actually could be solved. difficultyInteresting certainly is applicable ;-) However, there *are* the easy cases where moving a header is appropriate, but it has to be decided on a case by case basis keeping header scope in mind, and reviewers should be aware of the underlying problem. [1] http://hea-www.harvard.edu/~fine/opinions/fiveminute.html