Bug 109781 – Rename some files in WebCore/editing/

• Component: HTML Editing
• Status: RESOLVED LATER

[Kent Tamura]

htmlediting.{cpp,h}   -> EditorCommandHelper.{cpp,h}
markup.{cpp,h}        -> MarkupHelper.{cpp,h}
visible_units.{cpp,h} -> VisiblePositionHelper.{cpp,h}

What do you think?

[Ryosuke Niwa]

(In reply to comment #0)
> htmlediting.{cpp,h}   -> EditorCommandHelper.{cpp,h}

I'd prefer calling it EditingHelper.{cpp/h} or EditorHelper.{cpp/h} since there are functions in that file used outside of editor commands.

> markup.{cpp,h}        -> MarkupHelper.{cpp,h}

Sounds good. Can we split StylizedMarkupAccumulator into its own file too?

> visible_units.{cpp,h} -> VisiblePositionHelper.{cpp,h}

I'd prefer VisibleUnits.{cpp/h} instead since line, paragraph, etc... are visible units.

[Alexey Proskuryakov]

I'm not sure if having "Helper" in file name is an improvement. What is the additional information provided by "Helper" part of the name?

If the longer name is not more helpful, I prefer a shorter one.

[Kent Tamura]

(In reply to comment #2)
> I'm not sure if having "Helper" in file name is an improvement. What is the additional information provided by "Helper" part of the name?
> 
> If the longer name is not more helpful, I prefer a shorter one.

Yeah, "Helper" is not so meaningful. However I think adding this word is helpful in this case.

* htmlediting.*
  Renaming them to editing/Editing.* would be confusing.  Editing.* sounds the main file in editing/ directory. 
  Renaming them to editing/HTMLEditing.* won't work. AFAIK, case-only renaming makes troubles.

* markup.*
  Renaming them to editing/Markup.* won't work.  Ditto.

* visible_units.*
  ok, I'd like to rename them to VisibleUnits.*.

[Kent Tamura]

Created attachment 190047 [details]
Cook on EWS

[EFL EWS Bot]

Comment on attachment 190047 [details]
Cook on EWS

Attachment 190047 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/16742378

[kov's GTK+ EWS bot]

Comment on attachment 190047 [details]
Cook on EWS

Attachment 190047 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/16745307

[Alexey Proskuryakov]

I prefer keeping existing names over renaming these files to "helpers". Later, someone will be able to come up with better names, or to refactor the code in a way that will make it possible to have good names.

[Darin Adler]

I actually like the current names for these files and I think we should just fix the capitalization.

For the markup file, the functions are the ones that create markup, otherwise known as serializing, the opposite of parsing, and also parsing functions. We use the word “markup” in the names of these functions. I think that Markup.h is actually a good name for the file and adding “Helper” is not helpful.

For htmlediting.h I think there is a reasonable argument for naming the file EditingHelpers.h. I don’t think it should be called EditorCommandHelper.h since these functions are useful outside of editing commands. And I do think that eventually it would be better to refactor so there is not a file of “helpers”, but for now this name matches the contents of the file. I think that HTMLEditing.h or Editing.h is a fine name and helpers does not add much.

So I would do:

htmlediting.{cpp,h}   -> Editing.{cpp,h}
markup.{cpp,h}        -> Markup.{cpp,h}
visible_units.{cpp,h} -> VisibleUnits.{cpp,h}

I agree in principle with Alexey’s suggestion to wait until we have even better names, but we’ve lived with these old capitalization ones for way too long.

[Alexey Proskuryakov]

I'm fine with correcting file name case, I was only objecting to adding "Helper" to the names.

[Kent Tamura]

(In reply to comment #8)
> For htmlediting.h I think there is a reasonable argument for naming the file EditingHelpers.h. I don’t think it should be called EditorCommandHelper.h since these functions are useful outside of editing commands. And I do think that eventually it would be better to refactor so there is not a file of “helpers”, but for now this name matches the contents of the file. I think that HTMLEditing.h or Editing.h is a fine name and helpers does not add much.
> 
> So I would do:
> 
> htmlediting.{cpp,h}   -> Editing.{cpp,h}

I don't like Editing.{cpp,h} because existence of Editor.{cpp.h} and Editing.{cpp,h} in the same directory would be very confusing.
I'd like to adopt EditingHelpers.{cpp.h} for now.

> visible_units.{cpp,h} -> VisibleUnits.{cpp,h}

Done in Bug 111426.

[Ryosuke Niwa]

(In reply to comment #10)
> (In reply to comment #8)
> > For htmlediting.h I think there is a reasonable argument for naming the file EditingHelpers.h. I don’t think it should be called EditorCommandHelper.h since these functions are useful outside of editing commands. And I do think that eventually it would be better to refactor so there is not a file of “helpers”, but for now this name matches the contents of the file. I think that HTMLEditing.h or Editing.h is a fine name and helpers does not add much.
> > 
> > So I would do:
> > 
> > htmlediting.{cpp,h}   -> Editing.{cpp,h}
> 
> I don't like Editing.{cpp,h} because existence of Editor.{cpp.h} and Editing.{cpp,h} in the same directory would be very confusing.
> I'd like to adopt EditingHelpers.{cpp.h} for now.

I would call EditingFunctions in that case. It's a little more descriptive than helpers.

[Kent Tamura]

(In reply to comment #11)
> I would call EditingFunctions in that case. It's a little more descriptive than helpers.

In WebCore, there are:
6 *Function.h
4 *Functions.h
8 *Helper.h
3 *Helpers.h
3 *Util.h
10 *Utils.h
0 *Utility.h
17 *Utilities.h

[Ryosuke Niwa]

Sounds like we're very inconsistent here :( I guess utilities is not that bad either but not as descriptive as functions. Maybe ap or darin has some opinions on this.

[Darin Adler]

There are many other files that don’t use these suffixes and have better names. Files like HTMLParserIdioms.h that could easily have had a “utility” or “helper” name. I think that newcomers to the project often want to add files with these names because they are used to doing so in other projects. I would strongly resist doing this more. Further, there is no way to count the many files that don’t have these suffixes, but that someone new to the project would have been tempted to name with one of them.

Given the huge number of files in the project, these counts are surprisingly small, and reflect the leakage of these files into the project, with people not aware of this issue.

I don’t think the numbers here give us insight into our standards for file naming.

[Ryosuke Niwa]

I agree. We shouldn't be naming more files helpers, utilities, etc...

[Kent Tamura]

We don't have new name candidates except case-only changes and confusing Editing.*.
I gave up to proceed this.