[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH/RFC] Replace RichEdit with Scintilla in log dialog

From: Stefan Küng <tortoisesvn_at_gmail.com>
Date: 2007-10-17 19:28:58 CEST

Alexander Klenin wrote:
> Attached (unfinished) patch has following goals:
>
> 1) Remove duplicated functionality like
> CAppUtils::FormatTextInRichEditControl / CAppUtils::FindStyleChars,
> since it is already implemented in SciEdit.
>
> 2) Ultimately, allow to edit log message directly from log dialog,
> without the need for 'Edit log message' context menu.

But the editing should be disabled until the user either doubleclicks on
the control or presses a special button. Otherwise, accidental log
message changes would happen.

> 3) This, in turn, would allow to remove InputDlg, since it is used
> (almost) only here.
>
> It is not complete yet, because:
> 1) There is a small regression -- URL auto-recognition is lost.
> It must be re-implemented using custom regexp in SciEdit.
> I will do that, but it will require some time.

That's not a big problem.

> 2) Cleanup is not finished -- some old code should be deleted.
>
> 3) There is no way to save edited log message -- this of course will
> be added too.
>
> However, the change is already rather intrusive, so I am posting this
> to ask for opinions:
> 1) Is it a good idea? I obviously think that it is ;-)

Keep in mind that Scintilla is not fully unicode aware. We might get
into troubles with this - but then the commit dialog uses it too and we
haven't heard of any complaints there either.

> 2) Should I file an issue?

Yes please.

> 3) Should I create a branch, or is above-mentioned temporary
> regression in trunk acceptable?

No, just work on the trunk. It's much easier to review and test changes
there than on a branch.
As long as it compiles, commit it to the trunk.

Now, some comments about your patch:

> @@ -242,11 +252,6 @@
> m_hAddedIcon = (HICON)LoadImage(AfxGetResourceHandle(),
> MAKEINTRESOURCE(IDI_ACTIONADDED), IMAGE_ICON, 0, 0, LR_DEFAULTSIZE);
> m_hDeletedIcon = (HICON)LoadImage(AfxGetResourceHandle(),
> MAKEINTRESOURCE(IDI_ACTIONDELETED), IMAGE_ICON, 0, 0, LR_DEFAULTSIZE);
>
> - // if there is a working copy, load the project properties
> - // to get information about the bugtraq: integration
> - if (m_hasWC)
> - m_ProjectProperties.ReadProps(m_path);
> -
> // the bugtraq issue id column is only shown if the bugtraq:url >
or bugtraq:regex is set
> if
>((!m_ProjectProperties.sUrl.IsEmpty())||(!m_ProjectProperties.sBugIDRe.IsEmpty()))
> m_bShowBugtraqColumn = true;

Why do you remove the property reading?

> @@ -574,6 +576,7 @@
> return TRUE;
> break;
> case SCN_STYLENEEDED:
> + {
> int startstylepos = Call(SCI_GETENDSTYLED);
> int endstylepos = ((SCNotification
> *)lpnmhdr)->position;
> CheckSpelling();
> @@ -582,6 +585,31 @@
> WrapLines(startstylepos, endstylepos);
> return TRUE;
> break;
> + }

Small styling issue:
case LABEL:
        {
                code;
        }
        break;

* always add a tab to indent after an {
* the break should be after the closing }

> + case SCN_HOTSPOTCLICK:
> + {
> + CString url, msg;
> +
> + TEXTRANGEA textrange;
> + textrange.chrg.cpMin =
> Call(SCI_WORDSTARTPOSITION, lpSCN->position, TRUE);
> + if ((lpSCN->position ==
> textrange.chrg.cpMin)||(textrange.chrg.cpMin < 0))
> + break;
> + textrange.chrg.cpMax = Call(SCI_WORDENDPOSITION, >
textrange.chrg.cpMin, TRUE);
> +
> + char * textbuffer = new
> char[textrange.chrg.cpMax - textrange.chrg.cpMin + 1];
> +
> + textrange.lpstrText = textbuffer;
> + Call(SCI_GETTEXTRANGE, 0, (LPARAM)&textrange);
> + //if (!::PathIsURL(url))
> + {
> + url = m_sUrl;
> + url.Replace(_T("%BUGID%"),
> StringFromControl(textbuffer));
> + }
> + delete textbuffer;
> + if (!url.IsEmpty())
> + ShellExecute(GetParent()->GetSafeHwnd(), > _T("open"), url,
NULL, NULL, SW_SHOWDEFAULT);
> + break;
> + }

You shouldn't put bugtraq: specific code in the scintilla control class.
That's something unrelated. Instead, just extract the url and send a
notification back to the parent window. Just like the rich edit control
does.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tortoisesvn.tigris.org
For additional commands, e-mail: dev-help@tortoisesvn.tigris.org
Received on Wed Oct 17 19:29:09 2007

This is an archived mail posted to the TortoiseSVN Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.