Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding clickable links to RichTextBoxTarget #13

Closed
mikhail-barg opened this issue Dec 11, 2015 · 7 comments
Closed

Adding clickable links to RichTextBoxTarget #13

mikhail-barg opened this issue Dec 11, 2015 · 7 comments
Labels
Milestone

Comments

@mikhail-barg
Copy link
Contributor

As you may have noticed, we are using RichTextBoxTarget quite extensively. It's very handy to keep track of all the actions visually. We also using the target to store information on exceptions happening during program execution. Still printing whole stacktrace (including inner exceptions) floods the control area, and is not that useful.

ex1

But not printing stacktrace is bad as well — it's often needed to identify the problem source. So I was thinking that it would be nice to display only short description (exception message) given that it's possible to show all the exception details on demand. I came with the following solution.

It's possible to create in-place links in the RichTextBox control, so I've patched RTB-target to create links after logging events that contain exceptions. Exception objects are stored in a dictionary inside the target.

ex2

Target also adds an LinkClicked handler to the control, and exposes it's own ExceptionLinkClicked event, having Exception object as parameter.

public Form1()
{
     InitializeComponent();
     RichTextBoxExceptionTarget.ReInitializeAllTextboxes(this);
     RichTextBoxExceptionTarget.GetTargetForControl(richTextBoxLog).ExceptionLinkClicked += Form1_ExceptionLinkClicked;
}

void Form1_ExceptionLinkClicked(object sender, Exception e)
{
    MessageBox.Show(e.ToString(), "Exception details", MessageBoxButtons.OK);
}

ex3

I'm actually very pleased with the results and I think that such functionality could be of use for others (it actually puzzles me that I haven't thought of this solution earlier). Still to be published it might need a complete re-vamp, like passing logEvent parameters to ExceptionLinkClicked handler. Or generalizing the ability to add links by allowing it for log events without exceptions somehow.

So I'm posting it here to get your opinion first. What do you think?

@304NotModified
Copy link
Member

I really like the idea! I think it could be very useful to first hide some details!

I'm curious about the following things:

  • what about memory usage if we keep storing messages in the dictionary? Should we restart things if it get too much?
  • can we configure everything from the (xml) config?
  • is a message box the most ideal for displaying the details?
  • it was not fully clear to me, does this only works with exceptions?

Further I would recommend to send as much possible info to the link clicked handler (at least the full logeventinfo)

Maybe it's also an idea to let the details be layoutable? So it can be full set from the config and you have full control to show.

@mikhail-barg
Copy link
Contributor Author

I'm very pleased that you liked my idea! Hope we could get it to release state!

it was not fully clear to me, does this only works with exceptions?

The original solution — yes, because it was what I designed it for. But I really feel this should be generalized. But I'm not sure — how exactly?

Maybe it's also an idea to let the details be layoutable?

Yes, I also thought so, but what would it be? A separate specific layout renderer? If yes, then would it be possible to show the link only for some specific messages? Probably with onexception layout renderer? But are there other options, if we decide it to be more generalized? Some other conditional layout rendereres based on logEvent params? Showing links for every line would be an overkill I think.

Another problem is how to attach logEventInfo to the link? For now I'm just parsing the clicked link text, getting integer exception id which I have to place there. It looks like "details N1", "details N2", which is not too great. It should be possible to have hidden urls for the links, but I was not able to make it work yet. Any help on this would be appreciated.

So I assume that it's possible to attach some kind of marker to the link, which leads us to following question:

what about memory usage if we keep storing messages in the dictionary? Should we restart things if it get too much?

Yes, for now I don't clear the dictionary, and it should be changed. I suppose it should be possible to detect what markers are being deleted when removing first lines on maxLines overflow, and then delete corresponding items from dictionary. Still this would require parsing RTF. Do you see a better way?

is a message box the most ideal for displaying the details?

Surely no, I'm showing this as a simplest example. For our purposes I designed a special form pretty-printing stacks and showing Exception.Data contents. Still I did not mean to bind any specific behaviour to link click event, therefore I'm exposing RichTextBoxExceptionTarget.ExceptionLinkClicked event (should be renamed if we generalize things), and allowing to get a target bound to specific control via static RichTextBoxExceptionTarget.GetTargetForControl(control) call. See my code above. I think it's not that bad design-vise.

can we configure everything from the (xml) config?

Besides my thoughts on layouts above, do you mean here that we could also bind a specific OnLinkClicked behaviour in the (xml) config as well? I'm not sure on this, do you have any proposals on how it could work?

@mikhail-barg
Copy link
Contributor Author

I also remembered another use-case for this feature. In one of our applications we had to switch to manual writing texts to RTB control instead of logging to be able to add links for some messages.

The application itself is about doing some transformations with an Excel file. We display some part of the file in a tabular listview. So when a processing routine finds some problematic cells it prints a message to a RTB including a link, click on which causes preview to focus on a problematic cell.

So this could work as a real-world use-case for using links without exceptions. I would really like to use NLog for logging there without missing link functionality.

@mikhail-barg
Copy link
Contributor Author

Ok, here's what I have been thinking on generalization and configuration of links. We surely don't want to add links to all messages being logged, we want to have ability to specify which messages have links and how links look like.

So I think we can achieve these goals by creating specific layout renderer, let's say ${rtb-link: link-text=Layout}. So only events that had rtb-link renderer active would have the links. The link text would be configurable by link-text parameter of the layout. And we would be able to use ${onexception} and ${when} conditional wrappers we would be able to show the links only when needed.

Now the problem is in how the renderer would actually work. First of all it would need to create some kind of token so the logEventInfo could be found when the link is clicked.
The only solution I was able to come with, is to have a global counter and store it in the LogEventInfo.Properties during renderer.Append call. Something like this:

[LayoutRenderer("hello-world")]
public class RtbLinkLayoutRenderer : LayoutRenderer
{
    internal const string PropertyName = "RTB-link-id";
    private static int s_id = 0;

    public Layout LinkText {get;set}
    protected override void Append(StringBuilder builder, LogEventInfo logEvent)
    {
        builder.Append(LinkText.Render(logEvent));

        int id = Interlocked.Increment(ref s_id);
        logEvent.Properties[PropertyName] = id;
    }
}

public sealed class RichTextBoxTarget
{
    private readonly ConcurrentDictionary<object, LogEventInfo > linkedEvents;

    protected override void Write(LogEventInfo logEvent)
    {
        ...
        object linkId;
        if (logEvent.Properties.TryGetValue(RtbLinkLayoutRenderer.PropertyName, out linkId))
        {
              linkedEvents[linkId] = logEvent;
        }
    }
}

So this looks a bit ugly (we are adding our custom property to user-space Properties) but it should work. What worries me is how to make the needed part of the text in the RTB control to be a link. The code I use now looks like this:

public static void InsertLink(this RichTextBox textBox, string text, string hyperlink, int position)
{
     textBox.SelectionStart = position;
     //Does not actually work ((
     //See: http://stackoverflow.com/questions/2850575/what-is-the-rtf-syntax-for-a-hyperlink
     //textBox.SelectedRtf = @"{\rtf1{\field{\*\fldinst{HYPERLINK ""mysuperlink_val""}}{\fldrslt{mysuperlink}}}}";
     textBox.SelectedRtf = @"{\rtf1\ansi " + text + @"\v #" + hyperlink + @"\v0}";
     textBox.Select(position, text.Length + hyperlink.Length + 1);
     textBox.SetSelectionLink(true);
     textBox.Select(position + text.Length + hyperlink.Length + 1, 0);
}
private static void SetSelectionStyle(this RichTextBox textBox, UInt32 mask, UInt32 effect)
{
     CHARFORMAT2_STRUCT cf = new CHARFORMAT2_STRUCT();
     cf.cbSize = (UInt32)Marshal.SizeOf(cf);
     cf.dwMask = mask;
     cf.dwEffects = effect;
     IntPtr wpar = new IntPtr(SCF_SELECTION);
     IntPtr lpar = Marshal.AllocCoTaskMem(Marshal.SizeOf(cf));
     Marshal.StructureToPtr(cf, lpar, false);
     IntPtr res = SendMessage(textBox.Handle, EM_SETCHARFORMAT, wpar, lpar);
     Marshal.FreeCoTaskMem(lpar);
}

So it seems that I cant just put some markup into the message text via layout renderer. I would need to call some methods on the RTB control to make the link magic work. And I don't see a way to do it with the layout renderer approach. Except to pass a lot of data via logEventInfo.Properties (actually a single object with all the needed data) and do the necessary processing in RichTextBoxTarget.Write. But this looks like an overkill.

What do you think?

@304NotModified 304NotModified self-assigned this Dec 14, 2015
@304NotModified
Copy link
Member

Ok, here's what I have been thinking on generalization and configuration of links. We surely don't want to add links to all messages being logged, we want to have ability to specify which messages have links and how links look like.

So I think we can achieve these goals by creating specific layout renderer, let's say ${rtb-link: link-text=Layout}. So only events that had rtb-link renderer active would have the links. The link text would be configurable by link-text parameter of the layout. And we would be able to use ${onexception} and ${when} conditional wrappers we would be able to show the links only when needed.

Sounds great!

Now the problem is in how the renderer would actually work. First of all it would need to create some kind of token so the logEventInfo could be found when the link is clicked.
The only solution I was able to come with, is to have a global counter and store it in the LogEventInfo.Properties during renderer.Append cal

You can also use LogEventInfo.SequenceID, or a new guid.

So it seems that I cant just put some markup into the message text via layout renderer.

I don't see that. You can generate a link text with a string? And we can render the layout to a string? We can render it before it has been clicked?

Or is it the problem that we need the whole LogEventInfo ? Maybe we could store it at the GDC/MDC or something like that?

@mikhail-barg
Copy link
Contributor Author

You can also use LogEventInfo.SequenceID, or a new guid.

Thanks for a SequenceID idea, I've used it in a latest rewrite! Also used GUIDs in another part.

@304NotModified
Copy link
Member

closed by #14

@304NotModified 304NotModified added this to the 4.2 milestone Dec 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants