Re: [PATCH] URL copied via "Copy URL to clipboard" results in error when pasted into Merge dialog

lördag 28 maj 2022 kl. 19:18:22 UTC+2 skrev Stefan:

> On Saturday, May 28, 2022 at 4:18:44 PM UTC+2 [email protected] wrote:
>
>> lördag 28 maj 2022 kl. 10:24:21 UTC+2 skrev Stefan:
>>
>>> thanks for all your patches!
>>> I went with option 4, I agree that's the best solution.
>>>
>>
>> (y), LGTM.
>>
>> I realise you reworked the patch a bit and removed GetWindowText 
>> altogether (I thought about it but figured there was probably a reason it 
>> was used). For the matter of consistency, should the same be applied to 
>> MergeWizardTree? Something along the lines of [1]?
>>
>
> now that you mention it: yes there was (and is) a reason for using 
> GetWindowText: GetString can return an empty string sometimes (e.g. when 
> using up/down to select from the history without hitting return). Not 
> always, but sometimes.
>

Oh.  Sometimes is never nice.

So I'm going to change this back to using GetWindowText again and then trim 
> the string from newlines.
>

LGTM, again.

Just one more thing <tm>.  In MergeWizardTree.cpp, you first check the URL 
using GetWindowText but when setting the m_urlFrom and m_urlTo you still 
get the URL using GetString(). So I would re-order things as in patch [1]. 

TortoiseMerge seems to look at Software\TortoiseMerge\ContextLines (from 
>> the settings dialog), which I havn't set, and thus gets a default value of 
>> 0 from src/TortoiseMerge/MainFrm.cpp line 2950. I'm attaching a proposed 
>> patch [2] to change this to three, which seems a better default. There were 
>> a few additional places in the code looking at the above reg key, but with 
>> a default value of 1 that I didn't change. I'm not sure if it makes sense 
>> to have different default values for the same registry key. The attached 
>> patch [3] should change the default setting in the Settings dialog to 3 as 
>> well. 
>>
>
> Thanks!
> I've applied your patches.
>

LGTM.

I will have a second look at TortoiseProc (as mentioned in the previous 
message) to see if I can make it emit a path base in to WC root instead of 
just the file name. I guess it might be difficult to do with TortoiseMerge 
since it basically compares two different files on the computer. It could, 
possibly, emit a path based in the WC root if either file is in a WC (in 
the patches in the first message, C:/Devel/tortoisesvn is a WC), but it 
will be impossible to do with the file from %TEMP%.
 

> If you would register at osdn.net I can give you commit access so you can 
> commit your fixes yourself :)
>

Login: danielsahlberg

Appreciate your confidence :-)
 
Kind regards,
Daniel

[1] MergeWizardTree_DontUseGetString.txt
 

-- 
You received this message because you are subscribed to the Google Groups "TortoiseSVN-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/tortoisesvn-dev/7d17d20f-ea96-411a-a5c3-4c51a6bf986fn%40googlegroups.com.

Index: MergeWizardTree.cpp
===================================================================
--- MergeWizardTree.cpp	(revision 29401)
+++ MergeWizardTree.cpp	(working copy)
@@ -202,6 +202,7 @@
     }
 
     CString sUrl;
+
     m_urlCombo.GetWindowText(sUrl);
     auto newlinePos = sUrl.FindOneOf(L"\r\n");
     if (newlinePos >= 0)
@@ -212,6 +213,10 @@
         ShowComboBalloon(&m_urlCombo, IDS_ERR_MUSTBEURL, IDS_ERR_ERROR, TTI_ERROR);
         return FALSE;
     }
+
+    m_urlCombo.SaveHistory();
+    m_urlFrom = sUrl;
+
     m_urlCombo2.GetWindowText(sUrl);
     newlinePos = sUrl.FindOneOf(L"\r\n");
     if (newlinePos >= 0)
@@ -223,11 +228,8 @@
         return FALSE;
     }
 
-    m_urlCombo.SaveHistory();
-    m_urlFrom = m_urlCombo.GetString();
-
     m_urlCombo2.SaveHistory();
-    m_urlTo = m_urlCombo2.GetString();
+    m_urlTo = sUrl;
 
     CString    sRegKeyFrom = L"Software\\TortoiseSVN\\History\\repoURLS\\MergeURLForFrom" + static_cast<CMergeWizard*>(GetParent())->m_wcPath.GetSVNPathString();
     CRegString regMergeUrlForWCFrom(sRegKeyFrom);