Re: TortoiseHg Shell Extension (again)
Adrian Buehlmann
On 14.06.2010 14:43, Marco Lizza wrote:
> Hello Adrian,
> back in office today I was using the updated shell-extension... and
> found out some minor leftover quirks. Most notably, the context menu
> shows up (without icons) if right-clicking on the start-button.
> Moreover, I'm not very satisfied on the fix I proposted two days ago.
> I'm testing a new (better) approach to solve the issue... will you
> appreciated another fix on the subject?
Sure. We always try to take fixes. I'm just a bit lazy at times
regarding the shell extension.
I have currently http://bitbucket.org/tortoisehg/stable/issue/1241 on
top of my shellext testing TODO list - in case you're interested in
helping out ("Dame-moji" far east problem - hand-waving to Yuki :-)
> Moreover, as a sidenote question, I've tried and used VS2005 to
> compile the shell-extension. I've applied some (minor) adjustments due
> to compilation errors. Should I send those fixes, too? Or is VS2005
> deemed "too old" for the development?
>
> Let me know. Bye for now.
We're using the gratis C++ compiler from the "Microsoft Windows SDK for
Windows 7 and .NET Framework 3.5 SP1" as explained in the developer
readme [1]. Command line build tools only.
I can't remember having seen any compile errors with that. But if
there's any error we're certainly interested in having it fixed.
[1] http://bitbucket.org/tortoisehg/stable/src/tip/win32/shellext/README.txt
Adrian Buehlmann
On 14.06.2010 15:52, Marco Lizza wrote:
>> Sure. We always try to take fixes. I'm just a bit lazy at times
>> regarding the shell extension.
>
> That's fine by me... in fact, I really enjoy myself in shell-extension
> programming at the moment. :P :)
>
>> I have currently http://bitbucket.org/tortoisehg/stable/issue/1241 on
>> top of my shellext testing TODO list - in case you're interested in
>> helping out ("Dame-moji" far east problem - hand-waving to Yuki :-)
>
> That seems fixed, but I'll have a code-review just to be sure.
>
>> We're using the gratis C++ compiler from the "Microsoft Windows SDK for
>> Windows 7 and .NET Framework 3.5 SP1" as explained in the developer
>> readme [1]. Command line build tools only.
>
> I've installed that PlatformSDK, too... and it perfectly works.
> However, I currently use both VS2005 and VS2008 (at home) and VS2008
> compiles perfectly with no errors. VS2005 is shipped with an "older"
> (pre-Vista) version of the PlatformSDK. That means that some
> Vista-specific types and declarations are missing (the ones regarding
> the GDI+ Aero-theme support). Moreover, the "MSI.lib" is missing, too.
> By means of some declarations and LoadLibrary/GetProcAddress calls,
> VS2005 is working perfecly. That's a nice "plus", from my point of
> view.
>
> Just to be clear, creating a VS project file and switching to the
> VisualStudio IDE is useless and unnecessary, in my opinion.
Marco Lizza
here they are the changes I was telling you about earlier today.
I kept them as small and isolated as possibile, in order to better
discriminate and import them in the main repository.
Currently I'm testing them both on Windows XP (x86) and Vista (x86).
Marco
Adrian Buehlmann
> Adrian,
> here they are the changes I was telling you about earlier today.
>
> I kept them as small and isolated as possibile, in order to better
> discriminate and import them in the main repository.
>
> Currently I'm testing them both on Windows XP (x86) and Vista (x86).
After having taken a quick look at the first patch: I'm not interested
in taking these patches, sorry.
I'm not interested reviewing patches that change that much just to add
support for VS 2005.
Marco Lizza
"you-have-to-install-the-right-platform-sdk" thing, after all (that
means VS2005 doesn't work only with the originally shipped
platform-sdk).
That means the whole bunch of patches will be discarded (only the
first one refers to VS2005)?
Please, let me know. Thanks,
Marco
Adrian Buehlmann
On 15.06.2010 07:52, Marco Lizza wrote:
> Right then, Adrian. I agreed with you, since it only a
> "you-have-to-install-the-right-platform-sdk" thing, after all (that
> means VS2005 doesn't work only with the originally shipped
> platform-sdk).
Right. If it would mean that much changes I'd rather not want to do it.
After all, we depend on Vista specific things anyway (at least that is
my current understanding with the special icon handling for the cmenu
for vista and later), so we should probably install a post VS2005
platform sdk anyway (which is recommended by the readme currently).
Sorry for not having made this clear before you did send the patches.
But I wasn't aware how much changes are needed.
Thank you for your patience, but I think I have to play a bit the bad
cop in this case here because we have quite a number of users already
and the shellext is not exactly as error friendly as the python parts of
TortoiseHg (where you get a nice traceback right away).
> That means the whole bunch of patches will be discarded (only the
> first one refers to VS2005)?
>
> Please, let me know. Thanks,
Do the other patches apply cleanly if the first patch is skipped? I
confess I haven't looked at them yet.
Now this could by a dog-fooding use case for you for using the mq
extension I've told you about. Mq allows you to strip changesets
(warning: will destroy history) and push and pop patches that are in the
queue. Mq ships with mercurial, it's just needs to be enabled (and
understood :-).
If you don't have mq enabled, you can use 'hg clone -r <base revision>'
and then apply (i.e. 'hg import') patch no 2 and onwards to the clone.
I think you also should consider starting to learn the patch email
function (needs patchbomb extension, which ships with mercurial. It only
needs to be enabled). Look at how others send patches to this list.
Marco Lizza
> After all, we depend on Vista specific things anyway (at least that is
> my current understanding with the special icon handling for the cmenu
> for vista and later), so we should probably install a post VS2005
> platform sdk anyway (which is recommended by the readme currently).
As for the context-menu,
> Sorry for not having made this clear before you did send the patches.
> But I wasn't aware how much changes are needed.
In fact, the needed changes concerns
1) the declaration o some Vista specific types and enumerations (that
are missing in the pre-Vista platform-sdk), and
2) the non static dependece from the "msi.lib" module (that is missing
in the the VS2005 shipped platform-sdk, most likely due to a mistake
made by Microsoft).
> Thank you for your patience, but I think I have to play a bit the bad
> cop [...]
Well, I appreciate that. :)
> Do the other patches apply cleanly if the first patch is skipped? I
> confess I haven't looked at them yet.
Yes, they do not depend from the first one at all.
That's the reason I split the modifications in several successive
commits. That way the first one can be skipped (it is the only one
that concerns with the VS2005 compatibility), as the rest of them aim
to improve the shell-extension itself.
> Now this could by a dog-fooding use case for you for using the mq
> extension I've told you about. Mq allows you to strip changesets
> (warning: will destroy history) and push and pop patches that are in the
> queue. Mq ships with mercurial, it's just needs to be enabled (and
> understood :-).
One more topic to dig into for me. :)
> If you don't have mq enabled, you can use 'hg clone -r <base revision>'
> and then apply (i.e. 'hg import') patch no 2 and onwards to the clone.
Yep, that's what I just did to (re)test the patches I proposed
yesterday, skipping the first patch-file.
By the way, that's awesome.
> I think you also should consider starting to learn the patch email
> function (needs patchbomb extension, which ships with mercurial. It only
> needs to be enabled). Look at how others send patches to this list.
Fine. I'll have a look at this, too! Thanks for the hint!
Marco
Adrian Buehlmann
>> Right. If it would mean that much changes I'd rather not want to do it.
>> After all, we depend on Vista specific things anyway (at least that is
>> my current understanding with the special icon handling for the cmenu
>> for vista and later), so we should probably install a post VS2005
>> platform sdk anyway (which is recommended by the readme currently).
>
> As for the context-menu,
>
>> Sorry for not having made this clear before you did send the patches.
>> But I wasn't aware how much changes are needed.
>
> In fact, the needed changes concerns
>
> 1) the declaration o some Vista specific types and enumerations (that
> are missing in the pre-Vista platform-sdk), and
>
> 2) the non static dependece from the "msi.lib" module (that is missing
> in the the VS2005 shipped platform-sdk, most likely due to a mistake
> made by Microsoft).
>
>> Thank you for your patience, but I think I have to play a bit the bad
>> cop [...]
>
> Well, I appreciate that. :)
Thanks.
>> Do the other patches apply cleanly if the first patch is skipped? I
>> confess I haven't looked at them yet.
>
> Yes, they do not depend from the first one at all.
>
Ok. I will look at your other patches then.
BTW, a good tactic in general is (if possible) to put the (potentially)
most controversial patches at the end of the series and have the clear
cut patches at the beginning.
You will learn this as soon as you start sending patches for mercurial.
Adrian Buehlmann
> On 15.06.2010 12:56, Marco Lizza wrote:
>>> Right. If it would mean that much changes I'd rather not want to do it.
>>> After all, we depend on Vista specific things anyway (at least that is
>>> my current understanding with the special icon handling for the cmenu
>>> for vista and later), so we should probably install a post VS2005
>>> platform sdk anyway (which is recommended by the readme currently).
>>
>> As for the context-menu,
>>
>>> Sorry for not having made this clear before you did send the patches.
>>> But I wasn't aware how much changes are needed.
>>
>> In fact, the needed changes concerns
>>
>> 1) the declaration o some Vista specific types and enumerations (that
>> are missing in the pre-Vista platform-sdk), and
>>
>> 2) the non static dependece from the "msi.lib" module (that is missing
>> in the the VS2005 shipped platform-sdk, most likely due to a mistake
>> made by Microsoft).
>>
>>> Thank you for your patience, but I think I have to play a bit the bad
>>> cop [...]
>>
>> Well, I appreciate that. :)
>
> Thanks.
>
>>> Do the other patches apply cleanly if the first patch is skipped? I
>>> confess I haven't looked at them yet.
>>
>> Yes, they do not depend from the first one at all.
>>
>
> Ok. I will look at your other patches then.
>
I lost motivation to look at your patches. Sorry. If someone else would
like to take over responsibility: please do so, it's all yours.
Basically, the shell extension is pretty much in "works for me" state
and is good enough for my purposes as it is.
Marco Lizza
> like to take over responsibility: please do so, it's all yours.
All right, then. Let's wait for someone to carry on the "validation". :)
> Basically, the shell extension is pretty much in "works for me" state
> and is good enough for my purposes as it is.
Fair enough. I'll go on with some minor contribution that have come to
my mind, when ready.
Marco