[Shelving] introducing Konrad Zuwala

[Julian Foad]

Hi Stefan & other Tortoise devs,

please welcome Konrad Zuwala in helping me with the Shelving UI in TortoiseSVN here at Assembla. Konrad has experience in Windows programming (which I don't) and used to use TortoiseSVN in his job so is pleased to be contributing to it now.

At the moment he and I are working through some basic UI fixes. More on those in separate messages.

- Julian

[Konrad Zuwała]

Hello guys! Nice to meet you! I hope I will be able to contribute with a good quality code :)

[Konrad Zuwała]

I will have my first question here regarding Shelve/Unshelve context menu. The problem that we found out was a situation, when we try to Unshelve when nothing is shelved. Right now, if you do it, after clicking Unshelve, nothing will happen. We thought about two solutions:

1. If nothing is shelved in a directory, just hide Unshelve,

2. Handle this situation in unshelve dialog (like show MessageBox("Sorry, nothing shelved")).

3. Is there something like ITEMIS_MULTIPLESELECTION?

First solution is quite problematic in a way it requires us to check in  CShellExt::Initialize_Wrap if we have anything shelved. We did it by adding  a method to SVN class -> HasShelvedItems, which would call SVN::ShelvesList and check if a list of shelved items is empty or not. However, we run into circular #includes -> we cannot include <SVN.h> in ContextMenu.cpp, as we're getting compiler errors. This also requires us to new ITEMIS #define -> ITEMIS_SHELVED, so kind of modifying "core" of Tortoise. 

Do you have any suggestions which way would be best to do it? Like, we go with number 2. solution, and handle it in dialog, or we try to hide Unshelve menu when nothing is Shelved?

As for 3., currently we want to disable Shelve, when either one or many items are selected - it gives confusing result. We want to do it by hiding Shelve menu when you have something selected. Detection of selection is easy, in CShellExt::Initialize_Wrap, there is pDataObj which is != NULL when we have selection. However, we must add states to itemStates telling that something got selected and later use this state in ShellMenuShelve/ShellMenuUnshelve. Is there such a flag (telling that something was selected by mouse) defined somewhere or we have to add it (ITEMIS_SELECTED)?

Thank you for help!

W dniu czwartek, 26 października 2017 15:26:13 UTC+2 użytkownik Julian Foad napisał:

[Stefan]

On Thursday, October 26, 2017 at 3:43:55 PM UTC+2, Konrad Zuwała wrote:

I will have my first question here regarding Shelve/Unshelve context menu. The problem that we found out was a situation, when we try to Unshelve when nothing is shelved. Right now, if you do it, after clicking Unshelve, nothing will happen. We thought about two solutions:

1. If nothing is shelved in a directory, just hide Unshelve,

2. Handle this situation in unshelve dialog (like show MessageBox("Sorry, nothing shelved")).

3. Is there something like ITEMIS_MULTIPLESELECTION?

First solution is quite problematic in a way it requires us to check in  CShellExt::Initialize_Wrap if we have anything shelved. We did it by adding  a method to SVN class -> HasShelvedItems, which would call SVN::ShelvesList and check if a list of shelved items is empty or not. However, we run into circular #includes -> we cannot include <SVN.h> in ContextMenu.cpp, as we're getting compiler errors. This also requires us to new ITEMIS #define -> ITEMIS_SHELVED, so kind of modifying "core" of Tortoise. 

Do you have any suggestions which way would be best to do it? Like, we go with number 2. solution, and handle it in dialog, or we try to hide Unshelve menu when nothing is Shelved?

I wouldn't do that if not really, really required. Because doing too much in a context menu handler is bad - imagine doing that on a slow network share where even fetching the status can take seconds...

If you want to do that, you have to move the SVN::ShelvesList() outside the SVN class: the shell extension is linked without any networking support, therefore requires compilation without any network svn libs - and the SVN class includes everything including the networking stuff.

Think of this as a way to go over ShelvesList() again and optimize it for speed so the context menu doesn't slow down :)

 

As for 3., currently we want to disable Shelve, when either one or many items are selected - it gives confusing result. We want to do it by hiding Shelve menu when you have something selected. Detection of selection is easy, in CShellExt::Initialize_Wrap, there is pDataObj which is != NULL when we have selection. However, we must add states to itemStates telling that something got selected and later use this state in ShellMenuShelve/ShellMenuUnshelve. Is there such a flag (telling that something was selected by mouse) defined somewhere or we have to add it (ITEMIS_SELECTED)?

Not sure what you mean: there's always something "selected" when showing the context menu!

Because the shell extension is only registered to be shown if an item/folder/directory/folder background is selected.

If you mean multiple items selected: there's the opposite flag ITEMIS_ONLYONE which is true if only one item is selected, false if multiple items are selected.

But as I said: there's always at least one item 'selected'.

Stefan

[Konrad Zuwała]

Well I mean by selection is explicitlty selecting items with mouse, not a click in folder. So, if you hit Ctrl and click on files, you will get them selected (it's what I mean by selection). Am I clear enough now?:)

[Julian Foad]

"Unshelve" is closely related to "Apply patch", but "Apply patch" has a special behaviour when one file is selected: it assumes that file is the patch file; and that doesn't make sense for "unshelve". So we wanted to just not show the "unshelve" option if a file (or a folder, or multiple items) is/are selected, because in that case the user may think the unshelve operation is somehow going to apply to (only) the selected item(s). Maybe later we will want to add the possibility of unshelving only the part of a shelved change that applies to particular (selected) file(s), but for now we don't have that.

So we thought it is best if it is available only when the user context-clicked in the folder background area, with no items explicitly selected/highlighted.

Can you advise how to do that, or do you think some different behaviour is better?

Thanks,
- Julian

[Stefan]

On Thursday, October 26, 2017 at 5:57:04 PM UTC+2, Julian Foad wrote:

"Unshelve" is closely related to "Apply patch", but "Apply patch" has a special behaviour when one file is selected: it assumes that file is the patch file; and that doesn't make sense for "unshelve". So we wanted to just not show the "unshelve" option if a file (or a folder, or multiple items) is/are selected, because in that case the user may think the unshelve operation is somehow going to apply to (only) the selected item(s). Maybe later we will want to add the possibility of unshelving only the part of a shelved change that applies to particular (selected) file(s), but for now we don't have that.

So unshelving works on folders. Or only WC roots?

If for folders, just use the same context menu struct and flags as for example the 'cleanup' command: it shows for folders that are under version control, but not for files or multi-selections.

If only WC roots, then copy the flags from the 'relocate' command. That only shows for WC roots. Use the ITEMIS_WCROOT flag.

Stefan

[Konrad Zuwała]

Hey Stefan, 

we got it set up properly.

However, one more question: I need to add a new string into the app. Looked how it's organized, but want to make sure I do it a proper way. String is "No shelved items!" -> when you try to unshelve on a folder that has nothing shelved, for now we just show MessageBox telling user that nothing was shelved, so cannot be unshelved. Later it will evolve, and probably will be handled by an UnshelveDialog, but for now we want to keep it that way. 

So, as I figured out, there's a file: TortoiseUI.pot. Should I just place my english string there, eg. IDS_NOTHING_SHELVED and it'll be used for autogenerating resources? Or there is some other way? I don't want to have this string in code :-) Also, we'll put just an english version of it, so probably it'll have to be translated someday, if we keep that solution.

Thank you!

[Stefan]

On Sunday, October 29, 2017 at 1:15:26 PM UTC+1, Konrad Zuwała wrote:

Hey Stefan, 

we got it set up properly.

However, one more question: I need to add a new string into the app. Looked how it's organized, but want to make sure I do it a proper way. String is "No shelved items!" -> when you try to unshelve on a folder that has nothing shelved, for now we just show MessageBox telling user that nothing was shelved, so cannot be unshelved. Later it will evolve, and probably will be handled by an UnshelveDialog, but for now we want to keep it that way. 

So, as I figured out, there's a file: TortoiseUI.pot. Should I just place my english string there, eg. IDS_NOTHING_SHELVED and it'll be used for autogenerating resources? Or there is some other way? I don't want to have this string in code :-) Also, we'll put just an english version of it, so probably it'll have to be translated someday, if we keep that solution.

You have to add the string in the visual studio resources. Go to the window "Resource View", then select "TortoiseProc" and expand it until you see the "string resources". Open those and you see the whole list of string resources. Go to the bottom and add a new one there with your IDS_NOTHING_SHELVED id.

That's all: the pot files are updated during the build from those resources.

Stefan