Re: [RFC] parser branches ready for commit
On Mon, Nov 22, 2021 at 11:26:14PM +0100, Dominik Vogt wrote: > I want to remove completely it once the code has been tested well > enough. It's just there for the moment in case something happens. OK. I'm now about to merge this to master. Thanks, Thomas
Re: [RFC] parser branches ready for commit
On Mon, Nov 22, 2021 at 06:16:02PM +, Thomas Adam wrote: > On Mon, Nov 22, 2021 at 12:02:33AM +0100, Dominik Vogt wrote: > > Mostly done, except a couple of comments where there's still work > > to do. > > OK, I'll wait for those. These won't go away anytime soon. First I'd need to figure out what their mening and context is. :-) > I've not noticed any crashes from running this for three days now. > > Note that I still don't think we should hide the debugging behind > '#define OCP_DEBUG 1' What do you mean with "hide"? > even though it's always set to on at compile time. > Certainly not for new code. > We could make it a BugOpts option if it really > mattered. I'm not saying we need to fix this now, mind you. I want to remove completely it once the code has been tested well enough. It's just there for the moment in case something happens. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [RFC] parser branches ready for commit
On Mon, Nov 22, 2021 at 12:02:33AM +0100, Dominik Vogt wrote: > Mostly done, except a couple of comments where there's still work > to do. OK, I'll wait for those. I've not noticed any crashes from running this for three days now. Note that I still don't think we should hide the debugging behind '#define OCP_DEBUG 1' even though it's always set to on at compile time. Certainly not for new code. We could make it a BugOpts option if it really mattered. I'm not saying we need to fix this now, mind you. Kindly, Thomas
Re: [RFC] parser branches ready for commit
On Sun, Nov 21, 2021 at 09:23:20PM +, Thomas Adam wrote: > On Sun, Nov 21, 2021 at 02:38:09PM +0100, Dominik Vogt wrote: > > The parser branch is a ready as it will be without people testing > > it more. The upstream branch dv/master hat the latest, merged > > together patches with extensive debug to stderr enabled. > > > > Please take a look. If there are no findings I'd like to put this > > on master. > > Before merging, it would be nice to: New version upstream on the dv/master branch (rebased). > * Convert some/all of the printf(stderr, ...) Done. > * Remove/convert some remaining comments starting "!!!". Mostly done, except a couple of comments where there's still work to do. > * Decide on the '#if 1' blocks Done, all gone. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
Re: [RFC] parser branches ready for commit
On Sun, Nov 21, 2021 at 09:23:20PM +, Thomas Adam wrote: > On Sun, Nov 21, 2021 at 02:38:09PM +0100, Dominik Vogt wrote: > > The parser branch is a ready as it will be without people testing > > it more. The upstream branch dv/master hat the latest, merged > > together patches with extensive debug to stderr enabled. > > > > Please take a look. If there are no findings I'd like to put this > > on master. > > I'll keep testing it here over the next few days. > > Before merging, it would be nice to: > > * Convert some/all of the printf(stderr, ...) calls to fvwm_debug() lines so > debugging is going through that mechanism. ... Yes, no problem. The debug printfs were meant to go away before a real commit, but that won't be happening soon. > * Remove/convert some remaining comments starting "!!!". The '!!!' is my way of marking the places that are incomplete or need work. It will take a while to get these done. I'd > * Decide on the '#if 1' blocks -- are there they to stay permanently? No. Any block marked with !!! must go away. I need to understand the single one that is not, but I think it must stay without the "if". > * Not just related to change per se (but still present) appears to be what I > would class as extraneous "return;" lines at the end of void functions which > seems odd to me. I always put a return at the end of a function. So, if there is a void function without a return, function is likely incomplete or buggy. > I will also get some tests in place as well. I thought we could have some litte suite: 1) A shell script that starts Xephyr on desk 99 or whatever. 2) Starts fvwm on it with "-f testconfig". 3) testconfig is a simple script that adds a bit of test framework like printing the a test name and providing some result testing command. 4) Tests that do they work in a standardised way using the framework. See the three attached parsing test cases for nesting, forking and self modifying functions. > > (Note: Produces a ton of debug output and is not suitable to the > > general audience. Debug output can be disabled by debug defines > > in functions.c and cmdparser_old.c.) > > It's things like this which tell me "it's not quite ready yet to be merged". > Which is good, because this branch is level with master so it doesn't need to > be merged just to test it -- and it's that shift in mentality, trying to > keep master in the most stable state it can be which helps a lot with the > release process. Yeah, agreed. Still sometimes it's necessary to get some help from the people who would test master but not obscure development branches. This mess of interleaving patches dealing with all kinds of topics is not what I'd accept nowadays in a stable branch (i.e. now that there's git). At the moment it can hardly be avoided because there are so many fundamental changes going on. Ciao Dominik ^_^ ^_^ -- Dominik Vogt # test nesting calls destroyfunc ftest addtofunc ftest + i ftest ftest ftest # test fork bomb destroyfunc ftest addtofunc ftest + i ftest + i ftest ftest ftest # test function growth destroyfunc ftest addtofunc ftest + i ftest2 destroyfunc ftest2 addtofunc ftest2 + i addtofunc ftest i ftest2 ftest ftest
Re: [RFC] parser branches ready for commit
On Sun, Nov 21, 2021 at 02:38:09PM +0100, Dominik Vogt wrote: > The parser branch is a ready as it will be without people testing > it more. The upstream branch dv/master hat the latest, merged > together patches with extensive debug to stderr enabled. > > Please take a look. If there are no findings I'd like to put this > on master. I'll keep testing it here over the next few days. Before merging, it would be nice to: * Convert some/all of the printf(stderr, ...) calls to fvwm_debug() lines so debugging is going through that mechanism. I know historically we've had different sections for certain debug (stack-ring, enternotify debug, etc.) but I am trying to move away from that, and if you turn on debugging, you get everything. This has helped supporting users already last year, in my experience. * Remove/convert some remaining comments starting "!!!". * Decide on the '#if 1' blocks -- are there they to stay permanently? * Not just related to change per se (but still present) appears to be what I would class as extraneous "return;" lines at the end of void functions which seems odd to me. I will also get some tests in place as well. > (Note: Produces a ton of debug output and is not suitable to the > general audience. Debug output can be disabled by debug defines > in functions.c and cmdparser_old.c.) It's things like this which tell me "it's not quite ready yet to be merged". Which is good, because this branch is level with master so it doesn't need to be merged just to test it -- and it's that shift in mentality, trying to keep master in the most stable state it can be which helps a lot with the release process. Kindly, Thomas
