From: Kazutoshi S. <k_s...@f2...> - 2010-03-18 17:31:11
|
dal...@us... wrote: > Log Message: > ----------- > Adjusted the ModeProvider so it is a little smarter in picking the mode for a file. (snip) This change seems have come from the exact motivation of the patch #2822686. https://sourceforge.net/support/tracker.php?aid=2822686 Please update the status of the patch. The comment from Matthieu, which concerns about performance seems also relevant for your change. > Modified: jEdit/trunk/doc/CHANGES.txt (snip) > +- Fixed ModeCatalogHandler so if an edit mode in the user-specific catalog has > + the same name as an edit mode in the system catalog, the version in the > + user-specific catalog will override the system default. This is what the > + documentation for writing an edit mode has stated is true for some time, now > + it is. (Dale Anson) Since CHANGES.txt is for users, it's not so good idea to start with a class name (implementation detail) here. It's better to write an entry to be easy to understand for users. Also, this looks like a bug fix. Would you like to submit a merge request for 4.3.x? But, because this revision also includes the following change (not a bug fix), this will need a separate branch. > +- Adjusted the ModeProvider so it is a little smarter in picking the mode for a > + file. It is possible for a file to match more than one mode. Previously, the > + first mode that would accept the file was selected as the mode for the file. > + This change checks a little more, so a mode that matches both the filename > + glob and the first line glob will be selected before a mode that matches only > + the filename glob. If no mode matches both filename glob and first line glob, > + then a mode that matches the filename glob will be selected before a mode that > + only matches the first line glob. This is particularly useful for xml files, > + so that Ant build files can be differentiated from jEdit actions.xml files and > + from regular xml files. (Dale Anson) Same as above. And this looks too long. I think the following is enough: Gave better priorities within modes which matches with a file at the same time, as the following order. - both the filename glob and the first line glob - only the filename glob - the first line glob > Modified: jEdit/trunk/org/gjt/sp/jedit/Mode.java > =================================================================== (snip) > + //{{{ equals() method > + /** > + * @return<code>true</code> if this mode has the same name as another mode. > + * Only the name of the mode matters, this allows user created modes to > + * be treated the same as global modes regardless of the file name glob > + * or the first line glob. > + */ > + public boolean equals(Object other) This looks a responsibility of class ModeProvider and should better be solved in ModeProvider itself. Additionally, both contains() and remove() over ArrayList based on equality needs O(N) time for N modes, which means whole set up needs O(N**2) time (call addMode() N times). This can likely result in a noticeable performance regression for hundreds of modes. If you want to find a mode by its name, the appropriate container would be Map<String, Mode>, which can search a mode with O(logN) time. > Modified: jEdit/trunk/org/gjt/sp/jedit/syntax/ModeProvider.java (snip) > + if (acceptable.size()> 1) { > + // most acceptable is a mode that matches both the > + // and the first line glob Missing "filename glob" before "and the ..."? > public void addMode(Mode mode) > { > + // Modes are loaded from the global catalog first, then from any > + // user catalogs. If a user mode is added with the same name as Only the caller knows the order of addition. Assuming the order on callee side is bad. Instead, I recommend to specify the behavior of addMode() on addition of equivalent mode, and put the comment about priority on caller side. -- k_satoda |