Subject: RE: Revised Report (maushah) - fdbk from RLechner on three files: (RL Ref: $CASE/bdelog2ks/FinalRptFdbk_rl2ms.2k0523) > -----Original Message----- > From: Maulik Shah [SMTP:maushah@cs.uml.edu] > Sent: Monday, May 22, 2000 1:48 PM > To: Joseph A. Francis > Cc: Joseph.Francis@fmr.com; Bob Lechner > Subject: Revised Report (maushah) > > Ref: $CASE/bdelog2ks/maushah/docs/myFinalReport.txt > Software Engineering I (91.523) Spring 2000 > Submitted by: Maulik Shah (maushah) > > Files modified- > /bde/src/bendpt.cc > /bde/src/bendptops.cc > /bde/src/captionops.cc <<<< RJL comments > /bde/src/dialog.cc > /bde/src/fileio.cc > /bde/src/hlink.cc > /bde/src/initClasses.cc > /bde/src/leftmenu_cb.cc > /bde/src/linkops.cc > /bde/src/nodeops.cc > /bde/src/select.cc > /bde/src/text.cc <<<< RJL comments > /bde/src/textops.cc <<<< RJL comments > > Ref: $CASE/bdelog2ks/maushah/genv11_0502/bde/src/*.cc ... > Ref: $CASE/bdelog2ks/maushah/genv11_0502/bde/src/captionops.cc > Log Report: $Log: captionops.cc,v $ > // Revision 1.2.2.3 2000/05/18 02:46:58 maushah > // Removed GENv6 condition. > // When selected_caption is set to the currentselection, the > // currentselection was not checked for NULL pointers, if currentselection > // = NULL, bde would freeze. > // Simplified if-condition loops. > Modifications made: > (line 183 of 736) > // unselect(); maushah 2k0502 > (line 211 of 736) > if(currentselection != NULL && selected_caption != currentselection){ > selected_caption = currentselection; > // maushah 2k0502 (added one condition in if-loop) > // When selected_caption is set to the currentselection, the > // currentselection was not checked for NULL pointers, if currentselection > // = NULL, bde would freeze. > (line 412 of 736) ... > /****************** Start code caption_fit_to_text********************* > // This method is designed to resize the caption so that it will fit > // around the text snug. It prevents the text from touching the border > // of the caption. It also, prevents too much white space between the > // text and border of the caption. > // > // Maulik Shah (maushah) 05/02/00 > > void caption_fit_to_text( str_data *data, char *globalfontname ) > { > char shape[10]; > hcg_key id; > > id = (hcg_key)currentselection->getid(); > pr_find(CG,CGid,id ); > strcpy(CGcurr->captionname,data->str); > strcpy(CGcurr->txtfont,globalfontname); > getDimensions(CGcurr->captionname,CGcurr->txtfont, > &CGcurr->txtwidth,&CGcurr->txtheight); > > strcpy( shape, currentselection->getshape() ); > > float center_x = currentselection->getcenterx(); > float center_y = currentselection->getcentery(); > float height = CGcurr->txtheight * 2; > float width = CGcurr->txtwidth + height; > > switch( shape[0] ) { > case 'C': // Circle > break; > case 'R': // Rectangle or RoundRect > CGcurr->txtoffsetx = CGcurr->txtwidth / 2 + 2; > CGcurr->txtoffsety = -CGcurr->txtheight / 2; > drawrectangular( center_x, center_y, width, height ); > currentselection->setradius( width, height ); > break; > case 'E': // Ellipse > break; > case 'I': // Input > break; > case 'O': // Output > break; > case 'F': // File > case 'B': // Buffer > CGcurr->txtoffsetx = CGcurr->txtwidth / 2 + 2; > CGcurr->txtoffsety = -CGcurr->txtheight / 2; > drawfile( center_x, center_y, width, height ); > currentselection->setradius( width, height ); > break; > } > > topobject->dorecalc((hcg_key)currentselection->getid()); > } > > ********* End Code caption_fit_to_text*************************/ > (Code is commented out since the functionality isnt completely resolved. > BDE crashes with sigseg11 and sigseg2 and aborts by saving. Hence I > commented the code until the bug is fixed.) > ======================================================================= > > Ref: $CASE/bdelog2ks/maushah/genv11_0502/bde/src/text.cc > Log Report: $Log: text.cc,v $ > // Revision 1.7.2.1 2000/05/17 22:43:40 maushah ... > (line 170 of 376) > /* extern int add_caption_name_flag; //maushah 05/02/00 */ > (line 172 of 376) ... Maulik - this doesn't make sense - it looks like you are changing large code blocks to update one specific data field at a time - with NO comments whatsoever to motivate or explain your design for these changes. I would never allow a programmer to be so tersely creative. Please do NOT check this in without supplying reasonable comments about how or why this particular design should work. > /* // maushah 05/17/00 Allow text to fit into caption > if( add_caption_name_flag ) { > trim(data->fontname); > load_font(data->fontname,xorGC); > XDrawString(XtDisplay(canvas),XtWindow(canvas),xorGC, > data->cur_x,data->cur_y, > data->cur_val,strlen(data->cur_val)); > trim(globalfontname); > load_font(globalfontname,copyGC); > XDrawString(XtDisplay(canvas),XtWindow(canvas),copyGC, > data->x,data->y,data->str,strlen(data->str)); > caption_fit_to_text( data, globalfontname ); > } else { ALthough you aren't expected to know this, inserting '_' was a poor fix to remove blanks from string fields NOT in last position (which are not parsable by pr_load). The right solution is to prohibit schemas which allow such fields except in last position. So the else case is deprecated and will eventually be removed again. Therefore, I do not see how it is the 'else' case of your new flag ??? > ReplaceSpacesInText(data->str,'_'); > strcpy(new_string, data->str); > currentselection->addattr(data->x,data->y); It looks like current_selection is ambiguous about its table type pre-condition - it's probably node or link or caption. text.c may be generic enough to cover all cases. > } > maushah 05/02/00 */ > (line 243 of 376) > (Code is commented out since the caption_fit_to_text functionality isnt > completely resolved. BDE crashes with sigseg11 and sigseg2 and aborts by > saving. Hence I commented the code until the bug is fixed.) > > Ref: $CASE/bdelog2ks/maushah/genv11_0502/bde/src/text.cc > Log Report: $Log: textops.cc,v $ > // Revision 1.7.2.2 2000/05/17 22:45:09 maushah > // Removed Genv6 condition and changed #if 0 to #ifdef 0 > // Removed unnecessary if-loop condition check > Modifications made: ... > (line 468 of 1163) > /*if(add_caption_name_flag ) { THis was truly a badly indented code block. You should shrink the 1-level indents to 8 bytes to minimize word-wrap and improve readability. > tempx = currentselection->getcenterx() - > GXcurr->txtoffsetx; > textData.cur_x = (int)(tempx); > tempy = currentselection->getcentery() - > GXcurr->txtoffsety; > textData.cur_y = (int)(tempy); > strcpy((char *)textData.cur_val,(char > *)HNcurr->nodename); Now why in the world is a node name referenced (and copied) inside of an apparent GX caption line update here? > textData.fontname = CGcurr->txtfont; > XmTextSetString( > XmSelectionBoxGetChild( textDialog, > XmDIALOG_TEXT), > CGcurr->nodename); Here comes nodename again! Perhaps this is another legacy code problem, that of copying old Node code into more recent Caption code to reuse it without attending to the renaming of identifiers, but it looks too suspicious for that since schema table data fields appear to get mixed up between tables. Are you sure that you didn't miss some code lines spanning two code blocks (HN/HA vs. CG/GX and merge these into one? > strcpy((char *)textData.str," "); > } > else > XmTextSetString( > XmSelectionBoxGetChild( textDialog, > XmDIALOG_TEXT), > ""); > // maushah 05/02/00 trying to fit text within a caption. > */ > (Code is commented out since the caption_fit_to_text functionality isnt > completely resolved. BDE crashes with sigseg11 and sigseg2 and aborts by > saving. Hence I commented the code until the bug is fixed.) > (line 549 of 1163) ... > Ref:$CASE/bdelog2ks/NEW_VERSION_bugs_ms.2k0413 > NEW_VERSION is used when BUILDING chgen; when chgen runs it controls > whether CODE to check on View name (subschema tables) and table version > #s is GENERATED in pr_*.c. (These are not in CVS - only > the Imakefile is.) > The followiong description is part of architectural design discussion that does not belong in $log revision comments. It should be replaced by a 1-parag. summary and referenced, not included in full in the source code. > pr_init precedes pr_load. pr-init parses a list of local *.dat files and > scans these files to verify that the specified version numbers by the > Views to be used did not contain overlapping pkeys; pr_init also finds > the largest pkey in use for each VERSION of each table in the View. > pr_add assigns consecutive pkeys after that. > > The version number part of this approach was abandoned because it is > incompatible with future cvs version management of bde *.dat files. But > -DNEW_VERSION disabled View as well as version checks. > Right now, in bde, pr_init (or pr_load) only scans the single opened file > and pkeys are only unique within that file. (Merging files with the same > tables but concatenation is only possible after translating pkeys of one > file so they don't conflict with the other.) > > The View part (selecting subgraphs of the ERD) is important to > selectively filter relevant tables e.g. for input and output Views during > schema converters like bde2htm, bde2sch, and bde2STD and > ....For now, it is disabled in bde as not relevant during diagram editing. > > I inspected gen_pr_add and found this comment re: NEW_VERSION that also > bypasses the version test: > -------- > > gen_pr-add.c line 19: > > inserted NEWVERSION flag for 0 version problem -CEM 19/11/92- > > gen_pr_add line 170: > > #ifndef NEW_VERSION > > fprintf(prload_fp, "\ > if (hcg_view_list.view_list[hcg_view_idx].version_list[hcg_tbl_idx] == ' > \\0')\n\ > return;\n\"); > #endif > > My conclusion is that (barring any other context that reverses the effect > of > NEW_VERSION definition) we only need to add -DNEW_VERSION to the > chgen run command in pr_util/Imakefile to bypass this undesired test. > (When genv11 runs, it also needs NEW_VERSION defined, as it is an > application of pr_util from the metaschema. The Makefile to > build gen11 already does this). > > I did a cvs diff -r bde2alpha_rv first, then cvs update; (if I don't > already have sticky bde/CVS/Tag = bde2alpha_rv then I need to cvs update > -b bde2alpha_rv bde first. That will make the tag stick to my directory > (confirm to check that bde/CVS/Tag = bde2alpha_rv). > Meanwhile, cvs co -r bde2alpha_rv $BDEROOT works to checked out and > build bde and add sticky tag bde2alpha_rv in your workspace. > End of long-winded design discussion that doesn't belong in source code. > *** TBD *** (Currently working) > Note: I haven't yet added comments about caption_fit_to_text in my report > yet, since the bug is just reported and not yet fixed. The effects would > reflect code changes in three files: > > $CASE/bdelog2ks/maushah/genv11_0502/bde/src/captionops.cc > $CASE/bdelog2ks/maushah/genv11_0502/bde/src/textops.cc > $CASE/bdelog2ks/maushah/genv11_0502/bde/src/text.cc > > Nodes and Captions are not the same. > Node text is in HN--->HA lines and Caption text is in CG--->GX lines. > > Captions and Nodes ARE both resizable - from center outward, which is > hard to control, because text does NOT stick to upper left corner but to > center during resize. The above is irrelevant here - but OK near the beginning to explain diffs in node and caption text argument passing.