Extra spaces with 'script_editor_argument' causes errors with VS

Posted by WillFa on Thu 15 Jul 2010 08:03 PM — 13 posts, 40,240 views.

WillFa USA Thu 15 Jul 2010 08:03 PM #0

Hey Nick,

I wanted to check out the Visual Studio 2010 IDE with Lua. using

SetAlphaOption ("script_editor_argument", "/edit '%file'")

so that devenv would recycle the running instance and add a tab. This causes an error in devenv that " /edit 'C:\Users\Me\Documents\Scriptfile.lua'" can't find that file and will not be added.

So it doesn't open the file.

Without /edit, you get multiple instances. /edit works from a cmd prompt.

The Shell can be DL'd from http://www.microsoft.com/downloads/details.aspx?familyid=8E5AA7B6-8436-43F0-B778-00C3BCA733D3&displaylang=en

If you were curious.

Nick Gammon Australia Forum Administrator Thu 15 Jul 2010 09:15 PM #1

Are you saying the problem is with "<space>/edit", or is the problem that /edit is not recognized? If the problem is just the space, can't you omit it?

WillFa USA Thu 15 Jul 2010 09:35 PM #2

Okay... looked at the code. Totally bogus guess to the problem before.

  // replace %file
  strArgument.Replace ("%file", strName);

  HINSTANCE hInst = ShellExecute (Frame, _T("open"), m_strScriptEditor, 
            CFormat ("\"%s\"", (LPCTSTR) strArgument),   // quote argument
            NULL, SW_SHOWNORMAL);

in doc.cpp

It's quoting everything.

"/edit C:\Users\Foo\Whatever\Blah.lua"

Twisol USA Thu 15 Jul 2010 09:42 PM #3

That does seem very plausible.

WillFa USA Fri 16 Jul 2010 08:58 AM #4

In theory


// replace %file
  strArgument.Replace ("%file", "\"%file\""); //Should really check that the user didn't include quotes himself.
  strArgument.Replace ("%file", strName);

  HINSTANCE hInst = ShellExecute (Frame, _T("open"), m_strScriptEditor, 
            CFormat ("%s", (LPCTSTR) strArgument),   // This is unnecessary, but I'm unsure if 
                //strArgument needs to be cast for ShellExecute.
            NULL, SW_SHOWNORMAL);

Would rectify the problem?

Twisol USA Fri 16 Jul 2010 10:13 AM #5

WillFa said:


CFormat ("%s", (LPCTSTR) strArgument),   // This is unnecessary, but I'm unsure if 
    //strArgument needs to be cast for ShellExecute.

It is definitely unneccesary. A CString has an implicit LPCTSTR conversion where applicable; the only reason it's explicitly cast in CFormat is because it uses the varargs construct, which isn't type-safe. You could almost certainly pass in strArgument without any extras and it would work.


// replace %file
  strArgument.Replace ("%file", "\"%file\""); //Should really check that the user didn't include quotes himself.
  strArgument.Replace ("%file", strName);

  HINSTANCE hInst = ShellExecute (Frame, _T("open"), m_strScriptEditor, 
            strArgument,
            NULL, SW_SHOWNORMAL);

Theoretically, yeah, I think that would do it.

Nick Gammon Australia Forum Administrator Fri 16 Jul 2010 10:17 AM #6

Twisol said:

It is definitely unneccesary. A CString has an implicit LPCTSTR conversion where applicable; the only reason it's explicitly cast in CFormat is because it uses the varargs construct, which isn't type-safe.

It's necessary because in a varargs situation it doesn't do the typecast, and it will crash if you don't explicitly cast. This is done all over the place in the code.

Twisol USA Fri 16 Jul 2010 10:22 AM #7

Nick Gammon said:

It's necessary because in a varargs situation it doesn't do the typecast, and it will crash if you don't explicitly cast. This is done all over the place in the code.

I'm talking about removing the CFormat(), because it just does "%s", making CFormat() unneccesary as well as the explicit typecast used therein, as my example shows. ShellExecute doesn't use varargs, and I literally just explained the typecast issue and why the explicit cast was used in your quote. :P

Nick Gammon Australia Forum Administrator Fri 16 Jul 2010 10:36 AM #8

Ah you see I read that in a linear way...

Quote:

Willfa: ... I'm unsure if strArgument needs to be cast for ShellExecute.

Twisol: It is definitely unneccesary.

So I took "it" to mean the cast.

Twisol USA Fri 16 Jul 2010 06:38 PM #9

Heh, it was a bit muddy all around. I took this:

CFormat ("%s", (LPCTSTR) strArgument)

to be the intended target of the "unneccesary" note, because the "%s" is redundant, so you can just pass strArgument.

WillFa USA Fri 16 Jul 2010 07:17 PM #10

Twisol read it right. I was commenting on the line of code.

I was unsure what CFormat returned, what strArgument was to begin with, and what ShellExecute expected; and I was too tired to look it up so I figured that it'd work as is and the comment would lead to Twisol or Nick knowing a more efficient way. :)

Nick Gammon Australia Forum Administrator Fri 16 Jul 2010 09:06 PM #11

Well OK I changed it to get rid of the quotes, but I should warn you that I found without them that things like Crimson Editor failed with a path like:


C:\Program Files\MUSHclient\scripts\test.lua

It tried to open two files:


C:\Program
Files\MUSHclient\scripts\test.lua

So I made it do this:


  CString strArgument = m_strScriptEditorArgument;

  if (strArgument.IsEmpty ())
    strArgument = "\"%file\"";          // default

  // replace %file
  strArgument.Replace ("%file", strName);

  HINSTANCE hInst = ShellExecute (Frame, _T("open"), m_strScriptEditor, 
            strArgument,   // quote argument
            NULL, SW_SHOWNORMAL);

So the default behaviour (if you don't use 'script_editor_argument') is to take the supplied file name, and quote it.

However if you use the 'script_editor_argument' feature, then quoting the appropriate part of the string is your job.

Twisol USA Fri 16 Jul 2010 10:32 PM #12

That sounds perfectly reasonable to me. :)