Opened 7 years ago

Closed 7 years ago

#125 closed defect (fixed)

When a Collection contains spaces in a collection only partial key is copied in Tools::Inspector2

Reported by: patrik.svestka@… Owned by:
Priority: major Milestone:
Component: default Keywords:
Cc: Also affects CVS HEAD (eXept version): no

Description

The issue is that when Tools::Inspector2 (rev: 17208:3e2915fd01ac) is used on a Collection. If the Collection contains spaces and user wants to use 'Copy Key' functionality only partial Key is copied (Copied to a first space found).

I have found out the issue when trying to load windows registry which contains spaces. Using the following code (note: The Altap Salamander must be installed for it to show something):

        |userMenuRegistryPath userMenuItemList registryPaths pathMappedToNamesAndValues|

        "registry"
        registryPaths                    := OrderedCollection new.
        pathMappedToNamesAndValues       := Dictionary new.

        userMenuRegistryPath := 'HKEY_USERS\S-1-5-21-119559289-1840127793-336618761-855951\Software\Altap\Altap Salamander 3.08\User Menu'.

        "Registry pathentry for Salamander's 'User Menu'"
        userMenuItemList := Win32OperatingSystem registryEntry key:userMenuRegistryPath.

        userMenuItemList subKeysDo:[:subKey | 
                             registryPaths add: (Win32OperatingSystem registryEntry key: subKey path)].

        registryPaths do:[:path |
            currentNamesAndValues := Dictionary new.  
            path valueNamesAndValuesDo:[:registryName :registryValue |
                     currentNamesAndValues at:registryName put:registryValue
            ].
            pathMappedToNamesAndValues at:path put:currentNamesAndValues
        ].
"/ Test single line quote
 
        pathMappedToNamesAndValues inspect.

Attachments (1)

issue_125_patch_1_of_1_r9417c310265a.patch (1.6 KB ) - added by patrik.svestka@… 7 years ago.
Copy the whole String in libtool

Download all attachments as: .zip

Change History (9)

by patrik.svestka@…, 7 years ago

Copy the whole String in libtool

comment:1 by patrik.svestka@…, 7 years ago

My first patch .], yay. Thank you for the guide Jan .].

I have tried to patch it with a simple patch. Taking all information and disregarding any Key structure.
(I have tried to implement similar logic as already implemented (Character space logic - which was flawed) - via regex(the current implementation is insufficient) and PEG(I do not know enough to imitate the regex behavior - so I have submitted just the simple patch).

comment:2 by jan vrany, 7 years ago

A lot simpler case to reproduce this is:

  1. Create and inspect dictionary a follows:
    Dictionary new
      at: 'one two three' put: 123;
      yourself
    
  1. Select key 'one two three' in the list on the left, then in context menu pick Copy Key
  1. Paste (Ctrl-V) anywhere

It should paste 'one two three' and not only 'one.


comment:3 by jan vrany, 7 years ago

I think we need to do it differently. Consider following dicrionary:

Dictionary new
  at: 'dark blue' put: Color blue darker;
  yourself

As it is now, copy key pastes 'dark which is wrong. With proposed patch, it pastes 'ldark blue' (#000073) which is also wrong ((#000073) is added by inspector).

I can't think or a reliable way how to "parse" the string and determine what has beed added by inspector and what's key object print string.

We can, however, figure out the key itself and return it's printDtring by overriding #selectedKeyName in DictionaryInspectorView like:

selectedKeyName
    | key |

    key := self selectedKey.
    key notNil ifTrue:[ ^ key printString ].
    ^ super selectedKeyName 

This seems to me as more robust solution. Would that solve the issue?

in reply to:  3 comment:4 by patrik.svestka@…, 7 years ago

Replying to jan vrany:

I think we need to do it differently. Consider following dicrionary:

Dictionary new
  at: 'dark blue' put: Color blue darker;
  yourself

I wanted to come up with the real-life example, the actual data I'm now working with. It took me longer than expected because I have encountered an invisibility issue (I'll open separate ticket for that).
My example (for which the solution below works too!):

Dictionary new
  at: 'Win32OperatingSystem::RegistryEntry(HKEY_USERS\S-1-5-21-119559289-1840127793-336618761-855951\Software\Altap\Altap Salamander 3.08\User Menu\1)'
  put: (Dictionary new
          at: 'Arguments'
         put:'-O -A "\\rm2ms001157\e$$\GR_Prisma_Interface"');
  yourself. 

As it is now, copy key pastes 'dark which is wrong. With proposed patch, it pastes 'ldark blue' (#000073) which is also wrong ((#000073) is added by inspector).

I can't think or a reliable way how to "parse" the string and determine what has beed added by inspector and what's key object print string.

This was the reason why it took me so long to come up with solution. I thought in a line that it is better for the user to have complete and added information than only partial.
Well I have found out a regex .+?(?=\(.*\).*$) or maybe .+?(?=\(.*\)$) which I would have properly to test it in StX but the Regex parser is unable to cope with this look forward regex.

We can, however, figure out the key itself and return it's printString by overriding #selectedKeyName in DictionaryInspectorView like:

selectedKeyName
    | key |

    key := self selectedKey.
    key notNil ifTrue:[ ^ key printString ].
    ^ super selectedKeyName 

This seems to me as more robust solution. Would that solve the issue?

Yes, this #selectedKeyName override fixes the issue even for the more complex example.

comment:5 by jan vrany, 7 years ago

Status: newtesting

OK, should be fixed in 83feab5a7b8d/stx.libtool

Last edited 7 years ago by jan vrany (previous) (diff)

comment:6 by jan vrany, 7 years ago

Ping. Could you test if current libtool (i.e., newer than 83feab5a7b8d/stx.libtool) works and if so, close this issue?

comment:7 by patrik.svestka@…, 7 years ago

The issue is fixed.

comment:8 by patrik.svestka@…, 7 years ago

Resolution: fixed
Status: testingclosed
Note: See TracTickets for help on using tickets.