Created attachment 148453 [details]
Add preferences panel

Created attachment 148454 [details]
Configure default page size

Created attachment 148455 [details]
Show/hide truncate table button

Created attachment 148456 [details]
Copy row values with headers (keyboard)

Created attachment 148457 [details]
Pack columns

Created attachment 148458 [details]
Show absolute row numbers

I've attached some patches that add the preferences panel and implement the options mentioned above. The patch set supersedes my previous patches for #156492 and #192270.

Hey mastero23, 

most important thing first: It's nice to see someone going into the code - so read the following comments with a positive subtext in mind. Another thing: I'm not a netbeans developer, so its not my call to make - I'm just expressing my opinion:

General:

I would recheck, if the additional context menus (row-number column, toolbar) are necessary. I would have normally not searched there.

Absolute row numbers:

The meaning of the number is even more overloaded than it was before. Sorting is (and most probably will stay) local to the current resultset. So once the user invokes the sorting function, the values only show an upper/lower bound within the resultset. While testing I used a slower query and saw an irritating display: When paging through the resultset, I see the numbers being updated before the resultset is updated - you bind the property change event on the DataViewPageContext which is updated before the resultset is updated. This also shows in the table header, but that is dimmed/disabled when paging, so you see a difference and most probably don't notice that page and data are out of sync.

Pack columns:

Looks like it does what it should. Only question: You are already on the EDT (expecting EventListeners to be invoked on the EDT should be save), so do you need the invokeLater protection/behaviour?

Copy row values with headers:

Looks good!

Show/hide truncate table button:

Looks good - see general comment abvoe.

Configure default page size

Does what it say. I would caution against setting to infinite there are already enough out-of-memory errors and this option _will_ cause more of them - in addition it takes considerable time.

Preferences panel:

For the default page size I would check whether it would be a good idea to enclose the three options into a panel and use a titled border for the header. For the first option maybe include an always: "Always pack columns".

HTH

Matthias

Created attachment 148536 [details]
Fixes

Thanks for your feedback. I added a new patch that includes your suggested changes to the preferences panel. It has to be applied on top of the others.

General:

I thought the context menus would be kind of handy, especially the one for the truncate button. But i don't consist on them, so should i just wipe them out?

Absolute row numbers:

To keep the update of the row numbers and the result set in sync, I'm now using a table model event listener instead of the property change event listener.

Pack columns:

If i don't call packAll with invokeLater, an IndexOutOfBoundException is thrown if I change the page size to a smaller value:

java.lang.IndexOutOfBoundsException: Index: 10, Size: 10
	at java.util.ArrayList.rangeCheck(ArrayList.java:635)
	at java.util.ArrayList.get(ArrayList.java:411)
	at org.netbeans.modules.db.dataview.table.ResultSetTableModel.getValueAt(ResultSetTableModel.java:245)
	at javax.swing.JTable.getValueAt(JTable.java:2717)
	at javax.swing.JTable.prepareRenderer(JTable.java:5719)
	at org.jdesktop.swingx.JXTable.prepareRenderer(JXTable.java:3537)

I've just started working with Swing, so i don't really understand the reason for this behaviour. Another solution would be to check the index in ResultSetTableModel.getValueAt. Any other ideas?

Configure default page size:

I understand your concerns about the infinite option. The reason added it is issue #156492, but there's no big deal removing it again.

Created attachment 148597 [details]
modified patch series

(In reply to mastero23 from comment #9)
> General:
> 
> I thought the context menus would be kind of handy, especially the one for
> the truncate button. But i don't consist on them, so should i just wipe them
> out?

I buy that for the truncate table button - as you said this is established (configuration of toolbar from context), for the absolute row numbers I would not do it.

> Absolute row numbers:
> 
> To keep the update of the row numbers and the result set in sync, I'm now
> using a table model event listener instead of the property change event
> listener.

I implemented a different approach (see attached ZIP). I bound the row offset info into the DataViewUITableModel - as that holds the data it seemed like the right place to put this. 

> Pack columns:
> 
> [IndexOutOfBounds Exception]
> 
> I've just started working with Swing, so i don't really understand the
> reason for this behaviour. Another solution would be to check the index in
> ResultSetTableModel.getValueAt. Any other ideas? 

Not really - While reviewing this I noticed, that I prevent a similar problem in the same way as you did. I had a quick look at the SwingX code that is used for the table/sorting infrastructure and this is ugly. The asumptions about event ordering made there are most probably wrong. I consider redispatching the packAll into the next EDT round not nice and its a visible artifact, but the alternative (rework all of the view code and probably parts of swingX) wont't get us anywhere.

> Configure default page size:
> 
> I understand your concerns about the infinite option. The reason added it is
> issue #156492, but there's no big deal removing it again.

It might be good to put guards into the loading routine to prevent excessive memory usage. As propoposal, I would let it stay for now. Someone from the netbeans team has to decide whether or not to risk this :-)