From: SourceForge.net <no...@so...> - 2012-04-30 08:32:50
|
Bugs item #3521413, was opened at 2012-04-25 12:49 Message generated for change (Comment added) made by kpouer You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100588&aid=3521413&group_id=588 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: None Group: minor bug >Status: Closed >Resolution: Fixed Priority: 5 Private: No Submitted By: Alan Ezust (ezust) Assigned to: Matthieu Casanova (kpouer) Summary: services.xml: no class or interface StatusWidget Initial Comment: In services.xml, we have some services defined like so: <SERVICE CLASS="org.gjt.sp.jedit.gui.statusbar.StatusWidget" NAME="multiSelect"> import org.gjt.sp.jedit.gui.statusbar.*; new MultiSelectWidgetFactory(); </SERVICE> but there is no "StatusWidget" class. Also, it is not clear whether one should implement the StatusWidgetFactory or the Widget interface. Perhaps the "name" of the service should be the same as the java interface that must be implemented. ---------------------------------------------------------------------- >Comment By: Matthieu Casanova (kpouer) Date: 2012-04-30 01:32 Message: Fixed in rev 21616, I don't think we need to merge this in 4.5 as it is not really a bug. ---------------------------------------------------------------------- Comment By: Alan Ezust (ezust) Date: 2012-04-28 13:41 Message: ok, in that case it must be fixed in Core and all plugins. I don't think anyone but kpouer has put them into plugins yet, so he should be the one to do it. Changing priority back to 5. ---------------------------------------------------------------------- Comment By: Jarek Czekalski (jarekczek) Date: 2012-04-27 02:56 Message: Since it is no longer "helloworld" I change my vote. I think it's better to have the class name correct. At cost of breaking unknown plugins. So it should be StatusWidgetFactory, as it is in StatusBar.java: StatusWidgetFactory widgetFactory = (StatusWidgetFactory) ServiceManager.getService("org.gjt.sp.jedit.gui.statusbar.StatusWidget", name); If we don't change it, we may experience a situation in the future that core tries to get class from name and fails in case of the widget class. Looks like we are all on the side of the change, so I reopen the entry. ---------------------------------------------------------------------- Comment By: Matthieu Casanova (kpouer) Date: 2012-04-27 02:18 Message: In fact when looking at the code there is a method that would fail: in the ServiceManager in that case public static <E> E getService(Class<E> clazz, String name) But if the status bar do not use it, there is no problem. ---------------------------------------------------------------------- Comment By: Jarek Czekalski (jarekczek) Date: 2012-04-27 01:59 Message: I suggest no change. As you said: it's not java class but a service class. Not enough justification to break peoples' code, even if it's unpublished. Good documentation and examples would be enough for programmers. ---------------------------------------------------------------------- Comment By: Matthieu Casanova (kpouer) Date: 2012-04-27 01:24 Message: Yes, in fact the "class" field of the service is only a key, it could have been "helloworld", but that's right it is better if it is a the class or interface of the service. Those services must be a factory because our services are singleton and in the case of widgets we need different instances for each views. So that 's right the service class name should have been org.gjt.sp.jedit.gui.statusbar.StatusWidgetFactory I think it is still possible to change that without a major risk since I don't think that any plugin released a widget, and even if one did have a widget the only problem would be that it would not appear in the status bar, there would not be a crash. So should we change this name? ---------------------------------------------------------------------- Comment By: Alan Ezust (ezust) Date: 2012-04-25 15:29 Message: I was confused but I clarified things in the java docs. I am not sure if this is really a bug anymore. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100588&aid=3521413&group_id=588 |