Console getHeight() method returns 500
Project: | JNode Core |
Component: | Code |
Category: | bug report |
Priority: | normal |
Assigned: | Stephen Crawley |
Status: | closed |
I am trying to build a simple "page" command (analogous to "less" or "more"). Then I call getHeight() on the console object, it is returning 500 (the height of the console buffer) rather than the height of the PC screen. Here's a snippet of code:
Shell shell = ShellUtils.getCurrentShell(); try { console = (TextConsole) shell.getConsole(); } catch (ClassCastException ex) { err.println("Page is only supported with a TextConsole"); exit(1); } this.screenHeight = console.getHeight(); this.screenWidth = console.getWidth(); err.println("screen height - " + screenHeight); err.println("screen width - " + screenWidth);
First, am I right in thinking that console.getHeight() and console.getWidth() should be returning what I am expecting? The javadoc is (shall we say) unenlightening.
Assuming that I'm right, is there someone who understands this code who can offer some advice? My first idea was to create and override for getHeight() in PcScrollableTextScreen that returns the 'parentHeight'. But I have a nasty feeling that it would break stuff. The various TextScreen classes seem to call getWidth() all over the place.
So my second idea is to add 'getVisibleHeight' and 'getVisibleWidth' methods to the TextScreen interface, implement appropriately, and change TextScreenConsole to use these methods.
I'm going to hard-wire a screen height / width into PageCommand for now, but I'd like to fix this soon. So advice would be appreciated. Is my second idea the right way to go?
- Login to post comments
#1
Here is a patch that adds 'getVisibleHeight' and 'getVisibleWidth' methods to the TextScreen interface and changes TextScreenConsole to use them. The changes work for me, but if anyone can seen problems with this approach, please let me know. I intend to commit the patch in 2 days time.
#2
The visible width/height look OK for the text screen.
Don't we need a similar pair for the console or it's safe to change the current methods incompatibilly?
#3
I guess it depends if any application needs to be able to find out the height/width of the buffer if the console's screen is buffered. If it does, then we need a pair, and we need to change all existing calls to console.getHeight to console.getVisibleHeight as well. Maybe we should just do this anyway to allow for future applications.
Is there a better name than 'visibleHeight' for this property?
#4
Don't know if it's interesting, but you could add a 'Dimension getVisible()' methode just so that we follow the 'normal' procedures like in java.awt.Component.
#5
You probably mean JComponent.getVisibleRect().
I cannot support that because on one hand the console shouldn't depend on AWT and on the other hand the console doesn't need the same level of flexibility as the Swing components because the console is always fully visible if active. Partial visibility is not the case here.
#6
I agree with Levente.
And Martin's comment illustrates why my first choice of getVisible{Height,Width} for the names is poor. Can someone offer any alternative names? What do people think of calling the methods getScreen{Height,Width} and getBuffer{Height,Width} ... since that's what they really are. And maybe we should do the same the same for the TextConsole methods.
The other thing I'd like to say is that it would be a REALLY GOOD THING if someone who understands the TextScreen and TextConsole stacks could add comprehensive javadoc and implementation comments. I find them to be very hard to understand.
Finally, re this issue of AWT / JNode core dependencies .... would it be possible to move all of the AWT, Swing and related libraries out of the "core" project? That would make it impossible to create dependencies on AWT in the JNode core code. It would also make the "core" project smaller, which would ease compilation problems under Eclipse. What do you think Levente?
#7
I was also thinking about various solutions like sticking to the current names when we refer to the visible width and size and using names whith buffer in it when we want to return what current methods return for the scrollable consoles.
However if I think for a moment I foresee your next question which will go like what is the location of the visible rectangle inside the backing buffer, because you need it too for identifying the portion of the buffer contents which is visible. As you can see it all start getting worse, because you started exposing internal details of various console implementations.
But we could ask what is a console in fact? And how does it differ from a screen?
Before making any change to the API I suggest to you to take a look at the current console implementations and also to examples where they are used.
Then you will find out that actaully the problem that you want to solve (creating a pager) is already solved. See the TextEditor class which implements a small editor which has also a read-only mode.
The solution I used was to create a new console on top of the current one, this time without scrolling. I handled the scollong myself maintaining an internal buffer too because I needed this for the editing functionality. That class was quickly created about 2 years ago for adressing and urgent need for minimal text editing functinality under JNode. Neverthless it might show an example to you for working with consoles. This same classes is reused in cc for handling f3 and f4.
If you still want to create your own pager you might try using a new console with scrollable text screen, load the buffer with your data and it might turn out that the screen size is not needed because the console and screen handles scrolling internally.
I think before you want to change the API you need to clarify the concepts: Console, Screen and their variants.
On the AWT/Swing dependencies I can say that, we like it or not, the AWT and Swing are part of the core Java SE platform. Moving them to a different compile set from the rets of the core Java libs and from the rest of the JNode core are very difficult if possible at all within reasonable limits, due to various compile time and runtime interdependencies. For addressing the Eclipse compilation problems I might try to move out to a new subproject large parts of the classlib which are already handled as separate subprojects in OpenJDK too. But I should get there first, I hope in a month or so.
#8
I updated the sourcetree and noticed your PageCommand implementation. Seeing the exact problem you are facing I checked the codebase again and came to the following conclusion.
We better keep the existing methods and their semantics unchanged; to
AbstractPcTextScreen add the getDeviceWidth() and getDeviceHeight() methods and implement them in subclasses wherever they are needed, add getDeviceWidth() and getDeviceHight() to TextScreenConsole and implement it accordingly. The device dimensions refer to the underlying physical device regrardless to the intermediary virtual text screens and consoles.
#9
This is in reply to comment #7.
You really need to understand that I am trying to implement a pager, not an read only editor. I want to be able to page from a pipe as well as from a file. I don't want the pager to wait until it sees EOF on the pipe before it displays anything. I want the pager to be able to be able to jump to the end, to the start, page forwards and backwards, search forwards and backwards and so on. You cannot do these things on top of a normal text editor; that works on a complete file not a pipe, and the UI is all wrong. You cannot do these things by relying on a (fixed sized) terminal buffer.
I can maybe get some ideas from the TextEditor; i.e. the way it sets up its text screen. I'll take a look. But I would be very surprised if I can reuse TextEditor directly; see above.
I think before you want to change the API you need to clarify the concepts: Console, Screen and their variants.
Exactly; see my next comment.
#10
This follows by comment #9
You say to me go away and try to understand the TextConsole and TextScreen APIs. I say to you, they are not coherent, and to a considerable extent they are not comprehensible.
The root problem here is that the TextConsole API mashes together two different models. On the one hand, it mirrors the fundamental operations of the TextScreen; i.e. getting and setting the cursor character position, getting and setting characters and so on: the paint-on-the-screen" model. On the other hand it provides the model of a "console" as a rolling window on a sequence of lines of text, where the lines are provided by the output streams of one or multiple applications.
This is a problem because the two models conflict. Suppose I'm an application trying to use a TextConsole to paint the screen. If some other application (say the logger) decides to write to text console's output stream, this will put characters onto my carefully drawn screen at some random place, and maybe move the cursor. (OK ... I haven't tried this, but this is what it looks like the code would do.) On the other hand, suppose that I am a shell in a scrollable text console, and the user launches a paint-on-the-screen mode application. When control returns, I am potentially left with clearing up the screen after the app. But worse than that, the chances are that the app's screen operations will have left useless rubbish in the scrolling buffer.
On top of this, there is the "small detail" that while the TextConsole provides a bunch of "paint-on-the-screen" methods, it fails to specify what they do. Are the X,Y coordinates relative to the screen or the buffer? When I paint characters, will the cursor move? If I paint characters past the end of a screen line, will they wrap onto the next line? And so on.
Worse still, the actual behavior clearly of the TextConsole API clearly varies across the different implementations / combinations of the TextScreen and TextConsole. The fact that you had to create an extra layer console layer in TextEditor proves it, That is really, bad.
What to do?
Well, IMO the first thing to do is to divorce the TextConsole and TextScreen APIs. There really should not be "paint-on-the-screen" methods in the TextConsole API at all.
A virtual console should consist of a virtual text screen which supports the "paint" mode of working, and a completely independent virtual text console. The latter would have a scrolling buffer and support stream output and keyboard line-editing, and probably would use one or more TextScreen objects to implement a private virtual text screen. When application control switches between text console and text screen modes, the console stack would switch between the two virtual screens. So stream output while the pager was running would go to the (currently hidden) text console virtual screen, and pager output would not go into the text console's scrollable screen buffer.
Does that sound like a clean conceptual model?
#11
Stephen, I agree with you about the need to clarify these APIs and you're probably right in the need to separate the 2 concepts.
I have even tried to do heavy rework/refactoring in that part a couple of month ago but introduced big regressions and levente had to roll back my changes.
I still have the changes locally and I could sent them if you want (but they might be a bit outdated).
While I was trying to build the telnet server for jnode, I had to deal with Console/TextConsole and the related classes. Last time I ran it, that was still not working well. That's why I tried the above refactoring.
I will try to clarify my ideas on the subject by looking again at current code and the local changes I did few months ago and will come back to you.
If you have some UML (class) diagram, just a simple diagram or even a short description of classes/interface/method you think as foundation of concepts you mention, I would be proud to discuss further with you about what should be done there (and even if you don't have such document
Another related point :
It would definitely be great to have junit tests of all these APIs we are discussing here but it sounds like a hard thing to test (I hope it's not impossible). Ideally, these tests would even be runnable outside of jnode (with lower level being stub classes).
#12
In reply to:
"#9
Submitted by Stephen Crawley on Thu, 10/16/2008 - 14:51."
Thanks for telling me that, though I hoped that after my post #8 you would realize that it's completly needless.
In reply to:
"#10
Submitted by Stephen Crawley on Thu, 10/16/2008 - 15:58."
I still wonder what's wrong with asking a fairly experienced Java developer (who also happens to have commit rights too for this project) to take the time to read and understand the Java code instead of expecting me to explain it while, as you know it very well, I'm busy with a completely different area.
Now I can see that you spent time to read some of it, nearly as expected, but you also spent time to write it up which was not really expected. Neverthless this gives me the chance to learn that your understaning is still incomplete.
Probably the author of the text screen and console related code would appreciate your comments if those woudn't be so harsh and at places inaccurate. I, as basically the user of the console API and extender of its implementation with the graphical console and clarifications in the console manager, had the chance to face some problems of it and I also know that even the author of the code wouldn't mind some improvements in it.
For your possible console related activities in the future here are a couple of clues based on the issue of text consoles and screens as I understand it:
- the main problem is that there is a resource, the screen where the text is matariaized and this resource should be used by various components of the system for text output. So we need to coordinate access of multiple processes in the system to a shared resource.
- the console is a higher level abstraction over the screen which also incoprorates support for handling user input. In JNode we prefer to access screens via the console and not directly. This is motivitaed by the virtualization capacity of the console. If we program against the console then we can change the underlying screen implementation without breaking our code. Currently we have the text mode screen and a graphical text screen. In the future we could have a screen implementation with terminal capabilities for handling remote access similarly to what Fabien is working on in his telnet server.
- you pointed out correctly two styles of working with the console: the serial access and the random access. However I'm sure that in some way our console implementations should support both styles of accessing the console. The charva toolkit (that I integrated with JNode in early 2004 as my first significant contribution) is using heavilly the random access capabilities of the console and that should remain so if we ever want to make it possible to use a charva application in a remote terminal.
The hazard you mention is the result of concurrent access to the console first of all. When you have two threads accessing the console serially in a concurrent manner you experience a similar undesirable hazard.
So if you want to address the problem of hazard in the text output then you better look at the coordination of concurrent threads accessing the console instead of the various styles of using the console from one thread. Splitting up the console to two things according to serial access and random access would only add complexity to the code and to the suer code which want's to use both styles for cnevenience. It might be an option to provide limited views of the console reflecting the different styles when an application is using one style or the other. A similar pattern can ba identified in the relationship between the various streams and RandomAccessFile in java.io.
- access of various consoles to the underlying unique screen is managed by the console manager. The active console has access to the physical screen the others write to screen backing buffers. So if an application is using a dedicated console then the other applications in the background will not interfer with its output and there is no hazard. This is already working and used in JNode, but somhow you didn't notice it.
- currently the actual scrolling functionality is implemented in the screens which is only made available via the scrollable consoles to the user of the console.
I personally do not feel like reimpleminting a console in TextEditor which happend to become surprisingly small comapred to what it can do and how efficiently. I reject your comments of calling talking about this stuff in the way you do like really bad and so on. Please do not forget my fellow JNode developer Stephen Crawley that you alrady have a fair share in the by yourself stipulated conceptual mess of the consoles and text screens bacause you extended the related interfaces and APIs and you also added your vitual console concept, which asuming that a console is already an abstraction over screen and with suport to vitualization, has a shade of stacking virtualization layers one on top of the other, which might be justified or not but should be done only with a very good reason. And apparently you did this without a good enough understanding of the exitsing codebase. That was only an idea to consider before slapping into my face like if I was the unique author a whole piece of codbase to which I only made contributions.
So far I can conclude that you afford to freely call things "really bad" which actually do not match your view without even considering the views of coauthors and thinking for a moment about the fact that you might be inaccurate too.
Don't forget that people working on various components of JNode in the past and present have some skills and knowlegde too. And from my previous experience of working with them they would be pretty open for well thought out valuable suggestion for improving things. This is certainly true for the console where I was not alone in recognizing problems. I would like you to acknowledge before calling things really bad that actually the current console stuff is one of the parts of JNode where we can experience reasonable levels of functional code and performance if we put it in the context of the whole system. Definetly making it better and clearer are request which make sense.
Careful, honest analysis and good constructive ideas are always important for the future evolutin of JNode and are welcome.
From a JNode commiter I would expect a more fair approach to the lagacy code in the JNode codebase and to the other past and current members of the team. In principle one could work also without constantly creating tension and seeking how to contradict without pondering the simple fact that maybe the other person is not compleytely without fundation either and might has good reasons for sustaining the idea. Fruitless arguments are very detrimental and the time and creative energy could be much better spent on actually doing the task everyone has to move this project forward. Actually I would be much happier if I never had to write up this post at the first place.
For me JNode has been a very interesting and innovative part time project and a really inspiring professinal and intellectual challenge. The last thing I want is to turn this into flame wars and further down a nightmare.
Hereby I suggest a spirit of cooperation and understanding in our future work because I think that this is the only way for making this project succeed.
In reply to the comment of FabienD:
If we look at the original topic of this thread and the recent commit of Stephen Crawley then we can conclude that the original question was answered and probably if everyone agrees we can close this thread.
For discussing the possible improvements of the console API a separate forum topic would be better and for tracking the implementation of the console related improvements we could use one or more new issues of type feature request.
#13
Levente, I'm not going to respond to your personal remarks, except to say that they do you no credit whatsoever.
I'll prepare a large patch to address the problems I've raised and submit it for review. I won't waste any more of your time discussing them until then.
#14
Could you please create an UML representation of the design you are working on. At least a comprehensive class diagram and probably several diagrams for the more significant activities and sequences of method calls for features you consider important. I think this would provide a more appropriate basis for debating a design than based on a patch and Fabien would like it too.
#15
Patch has been committed. Marking this issue as fixed per Levente's comment #12.
#16
Automatically closed -- issue fixed for two weeks with no activity.