[Feature] Add new function guard to track entering leaving function in debug output.

(from [d970d6b283a13757d84dc447bb53993be353deb6]) Debug function guards: implement the guard.

• Implement the guard.
• Implement auxiliary "guard_output" macro that allow for properly indented comments.
• Set of unit tests for the guard and for guard_output

[#8 state:needs-ack]

Signed-off-by: Jocelyn Legault jocelynlegault@gmail.com
http://github.com/joce/npp-community/commit/d970d6b283a13757d84dc44...

The DEBUG_VERBOSE vs _DEBUG makes sense.
That could be useful to debug problems that would only appear in release or shipping.

Instead of DEBUG_VERBOSE, can we say !SHIPPING instead?

Thinking about it, I would hate to have this code bloat the distributed binaries. In any case, so far, the only difference between SHIPPING and NDEBUG (aka Release) is the tests that are compiled out.

Anything I should do about the reported vcproj conflicts?

How about both !SHIPPING and DEBUG_VERBOSE, solves the shipping binary and still gives finer control of verbose output.

No, you don't need to do anything regarding those two conflicts. They were expected.

Bah! Posted with the wrong account!

I'd like to hear how you intend to ues the guards so I can understand the need for DEBUG_VERBOSE better. The way I saw func_guard, they where to be added when debugging, and removed before submitting your branch.

I thought that was just a tool one would use punctually, like debugf().

If we were to sprinkle them all across the board, N++ would become unusable when they would be enabled. I've just enabled them in DockingCont and DockingManager, and N++ slows to a crawl.

If you want to leave the func_guard() calls in there, we would need a way to enable / disable per file.

Well, I do feel that having the tool available is a definite good thing, yet also, adding and removing would get quite tedious. Trying to enable/disable on a per file basis seems like it would be quite a job to setup properly given that n++ nor scintilla take advantage of namespace to any extreme.

Using a per function control file might be a way to go. A header with something along the lines of an ifndef #SHIPPING and ifdef #DEBUG_VERBOSE would then process a list of defines (say something like #define GUARD$function_name ) then developers wouldn't need to add remove from the code itself, it could easily be enabled/disabled purposefuly_ and it would still offer fine grained control...

What about that?

Type:: http://github.com/joce/npp-community/comments

test comment.

(from [de985f2ecc9a0053ee3b9dcff3eb0a65fee9adb8]) Merge branch 'jl/LH-8/debug-function-guards' into pu

• jl/LH-8/debug-function-guards: Debug function guards: implement the guard.

[#8 state:proposed]

Signed-off-by: Thell Fowler git@tbfowler.name
http://github.com/npp-community/npp-community/commit/de985f2ecc9a00...

A word of warning:
That feature should not make it in the trunk before 1.0.0 is out of the
door.

(from [97fd698d2300f8cb6f84ff2742a9ce3a0ccdff7f]) Merge branch 'jl/LH-8/debug-function-guards' into pu

• jl/LH-8/debug-function-guards: Debug function guards: implement the guard.

[#8 state:proposed] http://github.com/npp-community/npp-community/commit/97fd698d2300f8...

(from [e2986d0b930b52ab4acab6d290831a9c86ca3774]) Debug guards: add a way to enable / disable per category

• New way to enable / disable function guards per category. Such a category can be either a file name or of a different scope altogether.

• In non-shipping, the disabled guards still leave a skeleton in place, but the amount of executed code is minimal and should not be noticeable.

• In shipping, the guards are still removed altogether.

[#8 state:needs_ack responsible:npp-community]

Signed-off-by: Jocelyn Legault jocelynlegault@gmail.com
http://github.com/joce/npp-community/commit/e2986d0b930b52ab4acab6d...

How many columns do you want?
I personally feel that 80 is way to restrictive in this day and age.
I usually try to limit myself to 132 (while I clearly busted that in a few
place in there).
Would 132 chars be ok with you?

I agree that a strict setting of 80 is kind of crazy in this day and age. I try not to go too far above that myself unless there is a reason, and I have a visual right margin set at 100 so that two pages w/ consolas 8pt can be read without too much vertical scrolling on 1400xXXX screen.

I've found 120-130 works nicely for single page viewing. It's up to you. I just would like to have it at least have a 'similar' coding feel to the rest of the source.

(from [a6fd1a5a639853e360b8c849d752d8115119ae41]) Debug guards: add a way to enable / disable per category

• New way to enable / disable function guards per category. Such a category can be either a file name or of a different scope altogether.

• In non-shipping, the disabled guards still leave a skeleton in place, but the amount of executed code is minimal and should not be noticeable.

• In shipping, the guards are still removed altogether.

[#8 state:needs_ack responsible:npp-community]

Signed-off-by: Jocelyn Legault jocelynlegault@gmail.com
http://github.com/joce/npp-community/commit/a6fd1a5a639853e360b8c84...

(from [c3182c46609e8b3bf9e4729194def373bf6807a1]) Debug guards: add documentation

• Documentation added to explain what is the purpose of the function debug guards and their intended use.

[#8 state:needs_ack]

Signed-off-by: Jocelyn Legault jocelynlegault@gmail.com
http://github.com/joce/npp-community/commit/c3182c46609e8b3bf9e4729...

(from [1085f7e970ef25800b75f9d1c07356a6af53eded]) Debug guards: add documentation

• Documentation added to explain what is the purpose of the function debug guards and their intended use.

[#8 state:needs_ack]

Signed-off-by: Jocelyn Legault jocelynlegault@gmail.com
http://github.com/npp-community/npp-community/commit/1085f7e970ef25...

Line 134 of MISC/Common/Common.h is changed from DEBUG to _DEBUG, which means that instead of the developer controlling when to activate the NO_DEFAULT_CASE it is done simply by using the Debug configuration.

Perhaps this is a good target for an initial usage of the function guard group/category capability.

I think we will need to implement a test that ensures there are no enable/disable statements in committed .cpp files. Not sure how, but it could be made a part of the review process as well as the build process.

I don't agree. The enable / disable calls need to be there if we want the guards to stay in the code. You'll get compilation error otherwise.

Also, as I mention in the comments / documentation, the suggested current use is basically one category per cpp file. e.g. in foo.cpp, you'd have category foo and in other.cpp, you'd have category other.

Maybe I need to clarify this in the documentation.

Alo, what do you think of adding an IsDebuggerPresent() to limit the amount of processing that occures when running a debug or release build without the debugger attached?

There's very little that goes on other then outputting to the debug console. And if no debug process is attached, OutputDebugString already does nothing by itself. See http://msdn.microsoft.com/en-us/library/aa363362(VS.85).aspx

I think we will need to implement a test that ensures there are no enable/disable statements in committed .cpp files. Not sure how, but it could be made a part of the review process as well as the build process.

I don't agree. The enable / disable calls need to be there if we want the guards to stay in the code. You'll get compilation error otherwise.

Also, as I mention in the comments / documentation, the suggested current use is basically one category per cpp file. e.g. in foo.cpp, you'd have category foo and in other.cpp, you'd have category other.

Maybe I need to clarify this in the documentation.

The func_guard() statement needs to be there, but why would you exprect the enable/disable statement to be in committed files? A single define in the npp_debug.h to check if there are active func_guards being read in from a non-tracked header seems to be a solution that eliminates having people stepping on each-others toes with enable statements.

Also, what do you think of adding an IsDebuggerPresent() to limit the amount of processing that occures when running a debug or release build without the debugger attached?

There's very little that goes on other then outputting to the debug console. And if no debug process is attached, OutputDebugString already does nothing by itself. See http://msdn.microsoft.com/en-us/library/aa363362(VS.85).aspx .

Good to know!

• npp_debug.h line# 150 should read ...'the function guards' instead of 'debug guards'

Line 134 of MISC/Common/Common.h is changed from DEBUG to _DEBUG, which means that instead of the developer controlling > when to activate the NO_DEFAULT_CASE it is done simply by using the Debug configuration.

Actually, it's not a problem with with the _DEBUG macro being used (which is the right one to use). It's a problem with many default cases not being handled. This probably will end up being the proper behaviour, but we still need to investigate to make sure none of these issues are serious.

The _DEBUG macro will be replaced with the DEBUG one for the time being, but as per our IRC conversation, a new ticket should be created to deal with the NO_DEFAULT_CASES being triggered.

The func_guard() statement needs to be there, but why would you exprect the enable/disable statement to be in committed files? A single define in the npp_debug.h to check if there are active func_guards being read in from a non-tracked header seems to be a solution that eliminates having people stepping on each-others toes with enable statements.

That could be useful in some cases, but in many other, having a centralized category file will be rather cumbersome, and will cause full recompilations when adding / removing categories.

Also, for generic calls that can be done in more than one context, it makes much more sense IMO to have the enabling done at file level. Otherwise, you'd still have to have a context for that one file, but it would be declared in a foreign place. i.e. TinyXML. It is its own context. I don't see why the enabling of TinyXML guards should be done elsewhere than that, and disrupting the compilation of other files that have nothing to do with it.

Secondly, that would mean that we have a file that knows about all the project, from the top, but that could be included by the bottom. Allow me to explain: Even though we currently don't have any formal code hierarchy, we can agree that utility files like common.cpp are at the bottom of the pile and should not know anything about files implementing higher level concept, as the Docking*.cpp files for example. With the centralized concept you propose, common.cpp you be exposed, however indirectly, to docking concepts (the Docking category) that mean nothing to it.

So, while I'm not totally opposed to categories that have scopes broader that files, I think that should be layered, and we should be careful what we put and include where.

From irc::

[13:36:27] while the explanation is clear enough, I still do not like the idea of the enable/disable being a tracked file.
[13:37:10] enable/disable should never get committed to an enabled state
[13:37:18] that is true
[13:37:46] by having the enabler in the tracked file more checks are required by the developer and the maintainer to ensure they don't make it into committed code
[13:38:00] That is not ideal IMO
[13:39:03] all right, let me ponder on that a bit while I fix some emergency here.
[13:39:05] bbl.
[13:39:09] kk

(from [affc8250136273c35ae66e3b533d597fdda99295]) Debug function guards: implement the guard.

• Implement the guard.
• Implement auxiliary "guard_output" macro that allow for properly indented comments.
• Set of unit tests for the guard and for guard_output

[#8 state:needs_ack]

Signed-off-by: Jocelyn Legault jocelynlegault@gmail.com
http://github.com/npp-community/npp-community/commit/affc8250136273...

(from [9b0cc77d6746b33fdd95cef417b810722e386e26]) Debug guards: add documentation

• Documentation added to explain what is the purpose of the function debug guards and their intended use.

[#8 state:needs_ack]

Signed-off-by: Jocelyn Legault jocelynlegault@gmail.com
http://github.com/npp-community/npp-community/commit/9b0cc77d6746b3...

(from [fc77c066e74f3ce3c048d47d9a9d5b38bd59fbd7]) Debug guards: add a way to enable / disable per category

• New way to enable / disable function guards per category. Such a category can be either a file name or of a different scope altogether.

• In non-shipping, the disabled guards still leave a skeleton in place, but the amount of executed code is minimal and should not be noticeable.

• In shipping, the guards are still removed altogether.

[#8 state:needs_ack responsible:npp-community]

Signed-off-by: Jocelyn Legault jocelynlegault@gmail.com
http://github.com/npp-community/npp-community/commit/fc77c066e74f3c...

(from [bcb2c98d3c5464030d27ef083475eacc0adaed88]) Debug output: improve output performance and design

• Replace the outputF function with the DebugOutput object that allows to bundle up the outputs and flush them to the debug console only when ready.
• It also improves the testability of debug tools that need to output as you now only have to overload the output part of the class.
• DebugOutput comes with its set of tests.

• The existing function guards tests have been updated to reflect the new output method.

• Future improvements:

  • Instead of an extern variable to the global DebugOutput (yuk) we could implement a DebugOutput stack where we could push and pop DebugOutput objects. This stack would live in the application object.

[#8 state:needs_ack responsible:npp-community]

Signed-off-by: Jocelyn Legault jocelynlegault@gmail.com
http://github.com/npp-community/npp-community/commit/bcb2c98d3c5464...

(from [f395906707f5ff83e716fe4c99ee29ee7ea74924]) Merge branch 'jl/LH-8v3/debug-guards-enabling-per-file' into pu

• jl/LH-8v3/debug-guards-enabling-per-file: Debug output: improve output performance and design Debug guards: add documentation Debug guards: add a way to enable / disable per category Debug function guards: implement the guard.

Conflicts:

PowerEditor/src/MISC/Common/Common.h
PowerEditor/src/precompiled_headers.h

[#8 state:proposed]

Acked-by: Thell Fowler git@tbfowler.name
http://github.com/npp-community/npp-community/commit/f395906707f5ff...

(from [ed2008df1df33674c7ff537d192e2f8e326a557d]) common.h: split generic tchar defines from common functions

• Prep for function guard category inclusion. npp_debug.h requires the generics, and some common functions require npp_debug.h.

[#8]

Signed-off-by: Thell Fowler git@tbfowler.name
http://github.com/almostautomated/npp-community/commit/ed2008df1df3...

(from [5f86353deafc23cb9f56ceb22a61849716d22907]) common.h: split generic tchar defines from common functions

• Prep for function guard category inclusion. npp_debug.h requires the generics, and some common functions require npp_debug.h.

[#8]

Signed-off-by: Thell Fowler git@tbfowler.name
http://github.com/almostautomated/npp-community/commit/5f86353deafc...

(from [1f9f01b1bc97b9878f1f8dd23ed7f6b09d6d679a]) Merge branch 'jl/LH-8v3/debug-guards-enabling-per-file' into pu

• jl/LH-8v3/debug-guards-enabling-per-file: Debug output: improve output performance and design Debug guards: add documentation Debug guards: add a way to enable / disable per category Debug function guards: implement the guard.

Conflicts:

PowerEditor/src/MISC/Common/Common.h
PowerEditor/src/precompiled_headers.h

[#8]

Signed-off-by: Thell Fowler git@tbfowler.name
http://github.com/almostautomated/npp-community/commit/1f9f01b1bc97...

You forgot to add
http://github.com/almostautomated/npp-community/tree/5f86353deafc23cb9f56ceb22a61849716d22907/PowerEditor/src/MISC/Common/CommonGenerics.hCommonGenerics.h
to notepadPlus.8.vcproj

Looks like I used 8v3 instead of v4... fixed

(from [a75302c78043bc9a22f9e3a8586a952290bfcd3f]) Merge branch 'jl/LH-8v4/introduce-debug-guards' into next

• jl/LH-8v4/introduce-debug-guards: common.h: split generic tchar defines from common functions testNppDebug.cpp: add tests for guard debug statements npp_debug.h: add guard_debugf_cat() controlled by guard state gitignore: do not ignore PowerEditor/MISC/Debug Debug output: improve output performance and design Debug guards: add documentation Debug guards: add a way to enable / disable per category Debug function guards: implement the guard.

• Merge note: When merging a few manual changes are needed to ensure that common generic and DEFAULT_NO_CASE are properly handled.

Conflicts:

PowerEditor/src/MISC/Common/Common.h
PowerEditor/src/precompiled_headers.h
PowerEditor/visual.net/notepadPlus.9.vcproj

[#8]

Signed-off-by: Thell Fowler git@tbfowler.name
http://github.com/npp-community/npp-community/commit/a75302c78043bc...

I've added a fix for the .gitignore from this branch in regards to PowerEditor/src/MISC/Debug being ignored.

1) reverts commit ca84a05d75c10a37863a8983ebfbe4d6dc9cc734
2) Correctly adds the pattern to not ignore to the main .gitignore file and removes the .gitignore from the ...MISC/Debug .gitignore which should be there until such time as there are files there to actually ignore.

The commits can be viewed at:

http://github.com/almostautomated/npp-community/commits/jl/LH-8v5/i...

(from [b0534c08a38171c84f6c20bbaa403af11c8f3ef4]) npp_debug.cpp: [trivial] remove stale comment

• OutputF is not in use here anymore, instead g_debugOutput is.

[#8]

Signed-off-by: Thell Fowler git@tbfowler.name
http://github.com/npp-community/npp-community/commit/b0534c08a38171...