Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#5535 closed enhancement (fixed)

Save window pos and size with all text file

Reported by: humdinger Owned by: axeld
Priority: normal Milestone: R1
Component: Applications/StyledEdit Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

This is hrev35734.

It'd be nice to have StyledEdit's window position and size saved in another attribute with every text file.

Attachments (2)

SE_PersistentWnd.diff (2.5 KB) - added by x-ist 9 years ago.
Makes StyledEdit use attributes for storing window position and size.
SE_PersistenWnd2.diff (2.5 KB) - added by x-ist 9 years ago.
Second try, Now using BNode..

Download all attachments as: .zip

Change History (13)

comment:1 Changed 9 years ago by korli

Could it be overkill ? I mean if every document has the attribute added, it should be standardized somehow with other applications than StyledEdit.

comment:2 Changed 9 years ago by humdinger

It may very well be a candidate for a standardized attribute. I'm not sure it's useful for non-text documents, but that would be the decision of each application, if it's respecting (reading/writing) that attribute or not. It would enable different editors to open a document identical to any other editor. ATM, Pe and PalEdit write pe-info, I don't know if Gobe writes position/size attributes.

Orginally, I missed this functionality, because StyledEdit text files make for very nice ReadMe files, but would be even nicer, if they opened with the size/position intended by the author... :)

comment:3 Changed 9 years ago by anevilyak

To me that sounds less useful ; different editors have different default toolbars, default fonts, sidebars with a list of open files, etc., and as a consequence what was a good window size to display the whole document in one editor doesn't logically follow as good in another. In which case it would be preferable to me for the editor to simply pick its default size based on the content instead of being forced to an arbitrary window size/position that happened to work well for another app.

comment:4 Changed 9 years ago by humdinger

OK. Then no standardization and follow the Pe road and every app adding its own "pe-info" attribute (and thus be able to add more properties besides size/pos). The original ticket still holds: have StyledEdit add these properties as another attibute, like it already does with text formatting info.

Changed 9 years ago by x-ist

Attachment: SE_PersistentWnd.diff added

Makes StyledEdit use attributes for storing window position and size.

comment:5 Changed 9 years ago by x-ist

My first contribution to Haiku. I tried to embed the extension as seamless as possible. However, my knowledge of existing class infrastructure is small yet.

comment:6 Changed 9 years ago by stippi

Patch looks good in principle, thanks a lot! I plan to have a look at the StyledEdit code tomorrow to see if it fits well, since I am not familar with that code base at all... if no one beats me to it. :-)

BTW, BRect has a Width() and Height() method, and BNode, from which BFile inherits, has methods to load and store attributes. I think the code could be more elaborate than your version, though. So it's fine as is, even though it's a bit POSIXy. Hm... on second thought, you would save the close() call by using BFile.

comment:7 Changed 9 years ago by axeld

Indeed, though a BNode would be sufficient.

BTW, there are a few style issues you want to watch out for, for example "-1 == bytesRead" is especially mentioned as bad style in our coding style page.

Changed 9 years ago by x-ist

Attachment: SE_PersistenWnd2.diff added

Second try, Now using BNode..

comment:8 Changed 9 years ago by x-ist

Thanks for the comments. What about the second patch?

comment:9 Changed 9 years ago by axeld

Owner: changed from korli to axeld
Status: newin-progress

comment:10 Changed 9 years ago by axeld

Resolution: fixed
Status: in-progressclosed

Thanks! Looks almost good, in any case I've applied it in hrev36656.

Besides minor coding style issues like lines longer than 80 columns, and two blank lines between functions, there was one actual issue that I changed: the Haiku API does usually not return -1 on error, but returns the error directly (which are negative in Haiku). Therefore, you must never check against -1 (that would be a specific error, in this case B_ERROR), but only check if the return value is not B_OK, or in a case where the actually transferred bytes are returned, check for < 0.

In hrev36659 I've improved your patch to be endian aware - Haiku should be able to run on big endian platforms, so this is something to keep in mind for all on-disk storage.

Also, I'm now checking if the values read actually make sense, and fit on the screen. This is something one should always do when importing data from an external source.

Anyway, thanks for your patch!

comment:11 Changed 9 years ago by x-ist

Well, makes sense! The reason for checking against -1 was the use of fs_read_attr(..) in my first patch, which then was replaced by documentNode.ReadAttr(..) in the second patch. And the result of fs_read_attr(..) is reported to be -1 in the BeBook in case of an error. But anyway, my mistake :) Heading to other tickets!

Note: See TracTickets for help on using tickets.