task: need to reorganize the editor directory structure

Don't want to start on this 'til Joe's plaintext changes are checked in, since that's likely to affect any directory reorg. Marking dependency.

JIt's probably time to start talking about what the directory structure ought to look like (at least if this is Severity Critical). Any thoughts? Should rules go in a separate directory? Should plaintext go in a separate directory? (My guess is no, but I'll wait to see what Joe checks in) Do we actually need directory restructuring? I'm not sure what the goal here is.

I think the main goal is to separate composer from editor (as text widget) files. So the minimal stuff that should be done is to move all the composer-only files out of editor/base and into editor/composer/ (which I already made). If we want to go further, and separate out rules etc, that would be OK by me.

the rules should be pulled out as well

Just thought I'd mention that Joe and I also toyed with the idea about putting the transactions in a seperate dir.

For those lurking here: I have something that compiles and basically runs. I'm not ready to post diffs yet, because I need to update my tree, re-do the way logging is defined (right now we have it hardwired in the makefile, which obviously won't work when we have a hierarchy of makefiles; on Unix I'm making it a configure option, will need help with what to do on mac and windows) but here's the organization I currently have: editor/base: ChangeAttributeTxn.cpp IMETextTxn.cpp nsEditor.cpp CreateElementTxn.cpp InsertElementTxn.cpp nsEditorCommands.cpp DeleteElementTxn.cpp InsertTextTxn.cpp nsEditorParserObserver.cpp DeleteRangeTxn.cpp JoinElementTxn.cpp nsEditorService.cpp DeleteTextTxn.cpp PlaceholderTxn.cpp nsEditorUtils.cpp EditAggregateTxn.cpp SetDocTitleTxn.cpp nsInterfaceState.cpp EditTxn.cpp SplitElementTxn.cpp nsSelectionState.cpp IMECommitTxn.cpp TransactionFactory.cpp nsStyleSheetTxns.cpp editor/text: nsAOLCiter.cpp nsInternetCiter.cpp nsTextEditRules.cpp nsEditorController.cpp nsPlaintextDataTransfer.cpp nsTextEditUtils.cpp nsEditorEventListeners.cpp nsPlaintextEditor.cpp nsWrapUtils.cpp editor/html: TextEditorTest.cpp nsHTMLDataTransfer.cpp nsHTMLEditorLog.cpp TypeInState.cpp nsHTMLEditRules.cpp nsHTMLEditorStyle.cpp nsEditProperty.cpp nsHTMLEditUtils.cpp nsTableEditor.cpp nsEditorTxnLog.cpp nsHTMLEditor.cpp editor/composer: nsComposerCommands.cpp nsEditorShell.cpp nsEditorShellMouseListener.cpp A few comments: I tried, several times, to split the transactions out into a separate library, but that's a big job and it would be a lot easier to do that separately. There are lots of interdependencies between nsEditor.cpp and specific transactions (especially PlaceholderTransaction, but then TransactionManager depends on PlaceholderTransaction and includes the rest of the transactions in the same library) so they have to stay in base for now. TextEditorTest sounds like it should be in text/ but it actually tests the html editor. Should transaction logging go in composer rather than html? Should it go in text? I assume logging should be enabled by default in the unix builds, since it always has been in our makefile. Any other classifications I should revisit before starting to batch this up and make diffs?

These should be moved to editor/composer: nsEditorParserObserver.cpp nsInterfaceState.cpp This should go in base, I think: nsEditorController.cpp I think this is dead and should be removed: TextEditorTest.cpp Isn't this also fundamental and should be in base: TypeInState.cpp

TypeInState is only used to remember lists of html styles to apply or unapply to the next thing typed. As such, it properly belongs in editor/html. If at some point in the future we split into a minimal html vs whole nine yards html, it ca go in the minimal group.

you might as well get ready for the minimal verses the whole nine yards

I'll keep it in mind, but I'm not going to do a minimal/full split in this pass. That involves splitting nsHTMLEditor and making a new subclass and a new interface and changing the references in half the classes which currently refer to nsIHTMLEditor. I think that should really be a separate bug, especially since this one is currently marked critical for embedding whereas I don't think we actually have a definite list yet of what customers would want in a minimal vs. full html editor.

agreed, don't do that now, but just wanted you to have that frame of reference

I've moved the files Simon suggested (except TypeInState, per Joe's comment) and am attaching a diff of the changes to various editor files which go along with the move. This diff doesn't contain Makefiles or the new locations, only the changes that had to be made to the existing editor files.

Here's the current list of files moved out of base: public: nsIEditProperty.h text: nsAOLCiter.cpp nsInternetCiter.h nsTextEditRules.h nsAOLCiter.h nsPlaintextDataTransfer.cpp nsTextEditUtils.cpp nsEditorEventListeners.cpp nsPlaintextEditor.cpp nsTextEditUtils.h nsEditorEventListeners.h nsPlaintextEditor.h nsWrapUtils.cpp nsInternetCiter.cpp nsTextEditRules.cpp nsWrapUtils.h html: TextEditorTest.cpp nsEditorTxnLog.h nsHTMLEditor.h TextEditorTest.h nsHTMLDataTransfer.cpp nsHTMLEditorLog.cpp TypeInState.cpp nsHTMLEditRules.cpp nsHTMLEditorLog.h TypeInState.h nsHTMLEditRules.h nsHTMLEditorStyle.cpp nsEditProperty.cpp nsHTMLEditUtils.cpp nsIHTMLEditRules.h nsEditProperty.h nsHTMLEditUtils.h nsTableEditor.cpp nsEditorTxnLog.cpp nsHTMLEditor.cpp composer: nsComposerCommands.cpp nsEditorShell.h nsComposerCommands.h nsEditorShellFactory.h nsEditorController.cpp nsEditorShellMouseListener.cpp nsEditorController.h nsEditorShellMouseListener.h nsEditorParserObserver.cpp nsInterfaceState.cpp nsEditorParserObserver.h nsInterfaceState.h nsEditorShell.cpp

Seeking review of the changes before extending the build to the other two platforms. The actual code changes aren't really that large (mostly moving code around, and some warning fixes); we can get together and go over them in a visual tool, if that would be easier. Additional help testing would be greatly appreciated, since Linux doesn't show me undefined symbols until they're actually called. I'll definitely need Windows help, since I don't have a machine that can build Windows. (I can write some makefile.wins, but can't test them.) I may need Mac help if I have to create new projects for the new folders, or if my Mac decides to be flaky about building, and just in getting the files moved onto the mac with the right file type (I'll ask offline). Kin, can you help with the Windows build, or should I ask Anthony?

I forgot to mention the last directory: build: nsEditorRegistration.cpp nsTextEditorReg.cpp And almost forgot to attach the diffs to add editor logging as a a configure option. Turns out the log is only needed in the html and build directories, so I can take it out of Makefile.in in the other directories. Also I've changed the "EDITOR_LOGGING" references in configure.in and autoconf.mk.in to "EDITOR_API_LOG" to match the name already used in the editor files (I don't know why I had previously used the other name).

Kin has it building and working on Windows now. We moved a few more things around. I've attached the most recent diffs. Now it's just Mac and sr left to go ...

r=jf lots of good cleanup. a few notes: kin revamped the IsFoo() nsHTMLEditUtils functs. We should merge in his changes with yours. I don't understand why IsInlineNode() etc are gone. Now some of the code is messier, and the only reason I can think of for this would be to force error propogation, but the errors aren't being propogated in the patch, so I'm confused. If it's just style, I'd rather you not do it. I'm the one who has to maintain most of this...

I've merged with Kin's factoring -- moved his base routines into nsTextEditUtils and I call nsTextEditUtils::NodeIsType from the remaining nsHTMLEditUtils methods. Should I attach a patch for those two files? I also have nsComposerController moved to composer/src now; for some reason, when I integrated with the changes of the last week, things didn't work, so I had to work on those classes anyway, and I went ahead and moved nsComposerController. The IsInline stuff had to be factored because it lived in nsEditor, yet it incorporated a lot of built-in knowledge of html tags. Also, it was very confusing trying to keep straight the difference between IsBlockNode, NodeIsBlock and IsNodeBlock. What's an example of code which is now messier than it was before? Maybe we can fix it and make it easy for you to maintain without having the html dependencies in the base editor and without having the confusion about which methods can be called from where. It might just be an issue of choosing more appropriate names. I'll attach a patch for the current state, in my tree, of the files discussed in this comment, to make sure we're all on the same page.

/i saw places where: if (IsInlineNode(foo)) return PR_FALSE; became: PRBool isBlock; NodeIsBlock(&isBlock); if (!isBlock) return PR_FALSE; I might have the routine names wrong but the pattern is right. This kind of change seems gratuitous. There's clearly a loss of succinctness and there is no corresponding win.

Joe: in nsHTMLEditRules, I made local helper apps since IsBlock and IsInline were called so often, so as not to inflate the code. (I think some of the early diffs didn't have these helper apps; is it possible that you were looking at an early diff when you had that reaction?) If there are other files (which ones?) which make these calls often enough to need the shorthand, we can move the helper functions into nsHTMLEditUtils -- would that be better?

Phase 1 is checked in: the changes to the existing files, factoring into a couple of new files, and the new directory structure with unix/win makefiles. Phase 2, once we get the mac build working, will be to copy the files in the cvs repository and cvs remove the old locations.

Moving to 0.9.1.

Per discussion with Beth yesterday, we're going to hold off a while on checking in Phase 2.

The current plan is to try to get this in for 0.9.4 if we can get it tested on all platforms in time.

Here are some updated attachments for what needs to be done now: patches for all the Unix Makefile.ins, and a csh script to link or copy the files to the proper places. Windows makefile.wins need to be converted, and it needs mac love from someone, and testing on both windows and mac. This does not move any UI files. That should be done by someone who understands the UI build (I wasn't able to get it building in anything other than the top level directory) and is better done as a separate step, preferably under a different bug, since it isn't needed for embedding.

Nobody's looked at this yet, as far as I know. Obviously it's not going to make it for 0.9.4.

do we have a gameplan on this anymore? Am I holding up the works? Or are they held up without me, as it were...

Status (and sorry, I should have updated the status whiteboard accordingly): This bug is waiting for mac and windows testing of the latest set of patches. I think Kin was going to test Windows; I wasn't clear who was going to test Mac. Would it help to rewrite the reorg script in perl so it would be cross-platform? (I wasn't sure if translating to perl would help that, since there's still the filename issue, e.g. what to use for pathname separators when moving files.)

This bug has been open for 6 months. Is it critical work? Just like to know, now that we reorg'd: what the benefits are other than cosmetic. why this would be more important than critical bugs, or publishing. what milestone is realistic. is 0.9.5 (early October) realistic?

spam composer change

I've marked all the old diffs invalid, so the attachment list should be more correct now. I've also attached Kin's old Windows .bat file and his makefile.wins zip filefor posterity; they may need to be worked over a bit since the directory structure was changed since the first iteration. Charley's looking at it on Windows; Simon said he'd look at it on Mac. Simon, let me know if you want a tar file of the editor directory to avoid having to translate or hand-apply the scripts. It's 2.5M, so I probably shouldn't attach it here.

cmanske noticed a new file that wasn't being handled in my script, so I've updated it, and also fiddled a few things so it will work better when it's used to copy files in the cvs repository.

The attached script, run from a populated mozilla/editor, will do the reorg. I've made some changes to the mac editor project access paths; I'll check that in when Akkana is ready to land. Everything builds fine. My only quiblle at the moment is that a libeditor/public is created by the windows script that I adapted this from, but no files are ever placed there. Is this intentional? If we want to move public headers around then I have to touch the perl-based mac build system to change a path name for manifest execution.

Good catch: libeditor/public being created but empty is not intentional, and I've now removed it from my copy of the script. Okay, now I need an sr. Simon? Kin?

I need to eyeball joe's post-move layout on mac

Ugh. It turns out that none of the diffs apply any more, probably because of all the relicinsing foo. I've been unable to get an answer on when the real relicensing will land, but it sounds like it's probably at least going to be starting in the next week. So it's going to beat us to the punch and I'll need to make another set of diffs after relicensing lands (sigh).

sr=sfraser on the new directory organization.

Ok, if libeditor/public was just a typo then r=jfrancis. I have an updated editor.mcp ready to go and have tested the build process for the normal editor library and also the lightweight text-only editor library.

Checked in some of the makefiles (the ones for the directories that aren't built yet). Now we need to get leaf or another build person to help us move files around in the repository. Meanwhile, at Simon's request, I filed bug 101464 to cover moving the idl files, which we should also do some day.

Sigh. Seemingly the only person capable of moving the files in the repository has been away during the allowed 0.9.5 checkin window, and the deadline is today, so it looks like this gets put off yet again. Meanwhile, the new REQUIRES build system landed, so I've updated the Unix Makefiles and figured out the requirements. (I may have some extras, but at least the list is smaller than it was before.) I think Kin said that windows already used REQUIRES, so if we're really lucky we won't have to change the Windows makefiles for this, and we can pester Leaf to make this happen shortly after the tree reopens.

I'll do the repository copy today or tomorrow, and will update the bug when i'm done. Once the makefile changes are checked in (after getting a=drivers, now that we're frozen), the old file locations will need to be cvs removed.

Thanks! All the Makefiles are in except the main one under editor (to remove base and add libeditor and composer directories). I'll mail drivers and coordinate with Joe for changing the Mac project files when we flip the switch.

a=dbaron. Hopefully any regressions will be painfully obvious, but do try to get the right packaging changes in the first time. :-)

Incredibly enough, the patch is in! We are now building from the new directories. The old directories have not yet been removed (I'd like to make sure that everything works first) so I'm leaving the bug open for the job of removing the old files in base.

I've removed the old files in editor base. Finally closing the bug!

Akkana, this is such a monolithic bug fix...do you think you can verify this and mark verified-fixed? thanks.

Marking verified.