Re: 2 patches

Hi

On Tue, May 7, 2013 at 6:53 AM, Jasmin Dizdarevic
<jasmin.dizdarevic@gmail.com> wrote:
>
>
>
> 2013/4/29 Dave Page <dpage@pgadmin.org>
>>
>> Hi
>>
>> On Mon, Apr 29, 2013 at 4:56 PM, Jasmin Dizdarevic
>> <jasmin.dizdarevic@gmail.com> wrote:
>> > Hi Dave,
>> > thank you for committing.
>> >
>> > Attached are two "bug fixes" and an improvement.
>> >
>> > - Can not save settings if thoussep and decimal mark are blank
>>
>> Good catch - applied.
>>
>> > - Not copying column header if selecting just one cell
>>
>> I don't think that's appropriate - the copy format should be
>> consistent no matter how many cells are copied, and we do apply the
>> quoting rules regardless. It might make sense to have a "Use
>> quoting/headers with single cell" option though, to allow the user to
>> control the behaviour (though, that name sucks - we'd need a better
>> one). If off, we just copy the string of the cell and don't add any
>> quotes or column names. If on, you get the column names and quotes as
>> you would with multiple cells.
>
>
> What do you think about a dropdown box in the options dialog:
> Copy column names:   Never/Multiple cell selection/Always

That could work.

>> > Another thing is the ability to disable number formatting at all in
>> > query
>> > window. Might be useful.
>>
>> That one might be useful, but it needs some tweaking:
>>
>> - We don't typically have options on frmOptions for settings you can
>> toggle directly on frmQuery. Apparently that rule wasn't followed for
>> auto-rollback I notice though.
>
>
> I'm with you, it was not my prefered solution. Better would be a query
> window limited variable, which overrides the sys settings during one
> "session".

Maybe, but that's not done anywhere else at the moment (having a local
setting override a global one), so let's not invent a whole new way of
doing things now. We should either have it on frmOptions, or frmQuery.
frmQuery should be used only if there's good reason to expect this
option to be toggled often.

> I'm going to check this again. Do we have a doxygen documentation about the
> source code? This would make it much easier to dive into the source code?

No, we don't.

>>
>> - The menu position you've used is certainly wrong. It definitely
>> shouldn't be in the middle of the EXPLAIN menu items, and probably
>> shouldn't be on the Query menu at all. The View menu seems more
>> appropriate to me.
>
>
> I see. I'm going to change the position.

Thanks.


--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company