fix error report dialog

[pavoljuhas]

The current errorreportdialog.py has some serious problems:

1. Send Report button does not work, because we don't maintain the server for handling the form anymore.
2. link to the bug reports is obsolete and useless.
3. missing link to the diffpy-dev google group
4. the 2nd line does not wrap on wxpython 3.0. Lot of text is out of margin an invisible to user.

Proposed fix

1. Drop the error report form. Remove text boxes for email, summary and full description. Keep only the Error log text box.
2. Replace Send Report button with Copy Error Log. Make it copy the traceback enclosed in GitHub block quotations so it is easier to paste into GitHub issue.
3. Fix text wrapping and update the wording as needed. I suggest to use full URL-s instead of "here".
4. Update bug-report link to point to GH issues.
5. Add link to diffpy-dev Google Group.
6. Add button Google this Error and make it form a Google query from path-independent items in the traceback.

@diffpy/contributors - any comments? Any takers for this task?

[dragonyanglong]

Hi @pavoljuhas , I am happy to take this issue if no others work on it yet?

[sbillinge]

Thanks Long On Tue, Aug 8, 2017 at 3:17 PM longyang notifications@github.com wrote:

Hi @pavoljuhas https://github.com/pavoljuhas , I am happy to take this issue if no others work on it yet?

— You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub https://github.com/diffpy/diffpy.pdfgui/issues/24#issuecomment-321069395, or mute the thread https://github.com/notifications/unsubscribe-auth/AEDrUeYfwGhnQ91N9Ix-hSIb1GYSjfA4ks5sWMJWgaJpZM4OV5HE .

[pavoljuhas]

@dragonyanglong - please go ahead, thank you.

[dragonyanglong]

Hi @pavoljuhas , I dropped the text boxes you described in step 1. But I am a little stuck at step 2.

1. I didn't find "the Error log text box" on gui. I entered the Feature Request / Bug Report by clicking "Help-Request a feature / Report a Bug". After I deleting all the text boxes (email, summary and full description) you said in step 1, it shows as below. Where is the "the Error log text box"?

2. In step 2 you said, what does "Make the Copy Error Log button copy the traceback enclosed in GitHub block quotations so it is easier to paste into GitHub issue." mean? I am a little stuck here. Really appreciate your suggestions.

Thank you very much!

[pavoljuhas]

@dragonyanglong - error report dialog has two display modes. The first one opens when executed from the "Request a Feature" menu and the second one shows when program receives an unhandled exception. The second mode can be simulated by running

python.app -m diffpy.pdfgui.gui.errorreportdialog

I'd suggest to hide the "Copy Error Log" button when in menu-mode and resize the window to avoid the large blank space. In the error mode the window should be larger and should display the Error Log.

Also - please use wxGlade https://sourceforge.net/projects/wxglade when changing the dialog layout. We want to keep the design/errorreportdialog.wxg file in sync with the errorreportdialog.py source.

BTW - you may need to convert the StaticText in TextCtrl to get the text wrapping right, see https://groups.google.com/d/msg/wxPython-users/VnADGZrcKJY/foy1bUFNTmIJ. Here is a look with text wrapped in the second paragraph (old wxpython):

[dragonyanglong]

Hi @pavoljuhas , thanks for the explanation. A quick question about the workflow when I use wxGlade. How to keep design/errorreportdialog.wxg file in sync with the errorreportdialog.py source? The following examples describing the question looks long, but my question is straightforward, I think. Thank you very much!

When I use wxGlade to open errorreportdialog.wxg file, then change the dialog layout, then generate the python code. In the generated code, it only contains the GUI layout part code, no information about the events, etc.

Example1: now, I created the new button Copy Error Log in errorreportdialog.wxg, and bind it with onCopy event handler. Then, I generate the python code in wxGlade. Then, I write code about onCopy in the .py file (so that when user click the button, it will copy the traceback enclosed in GitHub block quotations):

    def onCopy(self, event):  # wxGlade: ErrorReportDialog.<event_handler>
        traceback = self.text_ctrl_log.GetValue()
        clipdata = wx.TextDataObject()
        clipdata.SetText("```" + "\n" + traceback + "\n" + "```")
        wx.TheClipboard.Open()
        wx.TheClipboard.SetData(clipdata)
        wx.TheClipboard.Close()
        event.Skip()

instead of the generated default code from wxGlade:

    def onCopy(self, event):  # wxGlade: ErrorReportDialog.<event_handler>
        print "Event handler 'onCopy' not implemented!"
        event.Skip()

However, this part of event handler code is not stored in .wxg file. As a result, next time, if I open .wxg file and do something new about dialog layout (like add a new button, change label, etc), I need to manually copy these code part from last time's .py file into the newly generated file. This kind of manual sync method would be painful because every time I generate code from wxGlade, I need to manually copy code part of events again, etc.

Example2: this might be more straightforward to my question. For example, I want to convert the StaticText in TextCtrl to get the text wrapping right (https://groups.google.com/d/msg/wxPython-users/VnADGZrcKJY/foy1bUFNTmIJ). I want to change the code in .py file:

text = wx.TextCtrl(self.panel, value=txt, 
style=wx.TE_READONLY|wx.TE_MULTILINE|wx.TE_NO_VSCROLL|wx.BORDER_NONE) 
text.SetBackgroundColour("ganisboro") 

instead of 
text = wx.StaticText(self.panel, label=txt) 
text.Wrap(text.GetSize().width - x) 

However, this change is not described in .wxg file, how to make .wxg and .py file sync in this case?

By the way, if I run python.app -m diffpy.pdfgui.gui.errorreportdialog, it cannot reflect my changes to the errorreportdialog.py file. When I run pythonm errorreportdialog.py, it can reflect successfully. Is there anything I did incorrectly?

[pavoljuhas]

How to keep design/errorreportdialog.wxg file in sync with the errorreportdialog.py source?

@dragonyanglong - to my understanding, wxglade changes only code between the # begin wxGlade and # end wxGlade markers such as this. So the workflow that keeps the wxg and py files in sync is to do GUI layout in wxglade and then generate the py code. This should change only the code between markers, other parts of the errorreportdialog.py source should stay the same.

Example 1 ... However, this part of event handler code is not stored in .wxg file.

The event handler code should be only in the .py file - it would be tedious to always edit it with wxglade. wxglade should add an empty handler function for new GUI items, but leave the handler alone once you update it afterwards. Perhaps there is a problem if you open the py file both in wxglade and a text editor. If you edit the py file, always restart wxglade before using it for GUI changes. That way the glade can reload the current py source. Also, check wxglade settings for any options that could prevent wrong code overwrites.

BTW - can you please rename that handler to onCopyErrorLog? It will avoid confusion with a GUI copy action.

Example 2 ... However, this change is not described in .wxg file, how to make .wxg and .py file sync in this case?

That code is between the wxglade markers, so you should open the wxg file and change the type of the label_text GUI item (if impossible, delete and re-insert as TextCtrl). Re-generate the py code and it should be updated as needed. BTW, any extra adjustments to the GUI items should be applied within the __customProperties method. Do not change the code between wxglade markers, because it will be lost in a next glade session.

By the way, if I run python.app -m diffpy.pdfgui.gui.errorreportdialog, it cannot reflect my changes to the errorreportdialog.py file.

Create a development conda environment, say dev27, activate it, and run

python setup.py develop -N

Check that pydoc diffpy.pdfgui shows a FILE value within your working repository. If so, the python.app -m diffpy.pdfgui.gui.errorreportdialog command will run from any directory and will use your working source code. Installing development version of PDFgui in a separate environment allows you to keep a working PDFgui in the default one - thus no problems if you break the development code.

Final note - it is good to use an editor that lets you compare a working version of some file with its commit in the git repo - that way you can easily restore any parts that wxglade did not get right.

[dragonyanglong]

Thank you very much! Pavol. The problem is solved. I think you are right. I used to keep the whole gui folder files open in editor when I generate the .py file, this is the why my sync doesn't work. And python.app -m diffpy.pdfgui.gui.errorreportdialog also run well now after following your instructions.

By the way, in my wxGlade v0.7.2, if I open the original design/errorreportdialog.wxg file from your repo without any editing, just generate the .py file. It is still different from your .py file. I screenshot the comparison below. (red is your .pyfile, green is my generated one without any editing)

1. Compare line 7, 8: "|wx.THICK_FRAME" is missing in my generated. Although in wxGlade, the style is checked correctly, but in generated code, it still doesn't show "|wx.THICK_FRAME".
2. Compare line 49 to 82 etc: most of the styles/Alignments etc are rearranged, although nothing is missing. And there will be space between |. This kind of problem happens all through the file.
3. Compare line 10 to 41: When I open your .wxg file. The IDs for all GUI items are default as wx.ID_ANY, not -1. I can fix this myself, because I can just manually set each label/text_ctrl/ button etc in wxGlade using ID: -1. Just let you know.

But for the first two problems, I cannot get an effective way to fix them yet. Perhaps it is due to different version of wxGlade in Mac and Linux? Also, just to provide more clues, when I open .wxgfile in wxGlade, it always show the warning as following: maybe this is the reason?

Thank you!

[pavoljuhas]

Thank you for a careful check. The py file is rewritten, because the original wxg design was done with an older wxglade and with wxpython 2.6. The current wxpython in conda is 3.0. wxGlade was also updated, so the generated code reflects those changes.

wx.THICK_FRAME is missing ...

That flag seems to be obsolete and gets applied with wx.RESIZE_BORDER - https://github.com/wxWidgets/Phoenix/issues/481

rearranged order of styles, an extra space in |.

The | operator in Python is for a bitwise OR; the order of arguments does not affect the result. An extra space around | makes the code more readable. You may want to use

git diff -w

to see only differences other than added or removed space.

-1 is replaced with wx.ID_ANY

wx.ID_ANY is defined as -1. The code does the same, but is more readable with wx.ID_ANY.

BTW, I would recommend to start your work with a separate commit, where you just re-generate the errorreportdialog.wxg and errorreportdialog.py with the recent glade. Use a commit message saying this is an update to wxGlade 0.7.2 and wxpython 3.0 with no changes to code function. The following commits should do the actual modifications to the dialog. That way it will be nicely separated what changes are for compatibility/style and what for function.

[dragonyanglong]

Hi @pavoljuhas , The followings (one for menu mode, one for error mode) are what I achieved so far. Any comments or suggestions before I submit PR?

Also, a few things may need your suggestions:

1. We will give both GitHub issues link and diffpy-dev Google Group link. Both of them are used as bug report / feature request. Shall we rewrite the wording here for user to better distinguish when to use GH issues and when to use diffpy-dev Google Group?

2. After rewrite the 2nd line, (remove short summary, full description etc), now the whole text is visible to user. But the following full URL-s lines (instead of here) become much longer than before. How about make each full URL-s as separate line? i.e.:

   
   You can view current bug reports and feature requests at GitHub issues:
   https://github.com/diffpy/diffpy.pdfgui/issues

Discuss PDFgui and learn about new developments and features at diffpy-users Google Group: https://groups.google.com/forum/#!forum/diffpy-users

You can also report a bug and request features at diffpy-dev Google Group: https://groups.google.com/forum/#!forum/diffpy-dev

2. About text wrapping (if we think we still need it?), I tried the same thing described in (https://groups.google.com/d/msg/wxPython-users/VnADGZrcKJY/foy1bUFNTmIJ), but it doesn't work as expected. For example, as you can see in the 2nd line in the above screenshot, the "fake" Text_Ctrl still doesn't look the same as normal label_text. I screenshot my setting for reference. How did you do that in your former comment above?
![image](https://user-images.githubusercontent.com/16789768/30131481-e830e57e-931a-11e7-99e3-52a0a52b3103.png)
![image](https://user-images.githubusercontent.com/16789768/30131487-ed517adc-931a-11e7-90c5-1dc1cb55a8ea.png)
![image](https://user-images.githubusercontent.com/16789768/30131492-f3290b00-931a-11e7-9ad9-39e2e6f980a2.png)

3. About `Google this Error` function, you want it to form a Google query from path-independent items in the traceback. So what I have done now, for example, is:
For the default testing error log, when user click `Google this Error`, it will only google search this line `StructureFormatError: 10: file is not in PDFFit format`.

Thank you!

[pavoljuhas]

Any comments or suggestions before I submit PR?

It is a good start, but the dialog is too wide, the second line is not well aligned and has some extra blue lines above and below and a white patch on the right. I think it is better to replace all header text labels with one wx.html.HtmlWindow where the content can be styled and mixed with hyperlinks and it also wraps when resized. Please see this gist for a prototype. If it looks OK to you, please incorporate into errorreportdialog using wxglade for design update. Note that some statements need to be moved out from the __init__ method of the prototype. Please, try to adhere to the existing conventions (i.e., which method does what) in the errorreportdialog source.

We will give both GitHub issues link and diffpy-dev Google Group link. Both of them are used as bug report / feature request. Shall we rewrite the wording here for user to better distinguish when to use GH issues and when to use diffpy-dev Google Group?

I agree it looks confusing. I think we should just remove the diffpy-dev link. It was added before the project moved to GH and had a public Issues page. @sbillinge - is that OK with you?

About Google this Error function, you want it to form a Google query from path-independent items in the traceback. So what I have done now, for example, is: For the default testing error log, when user click Google this Error, it will only google search this line StructureFormatError: 10: file is not in PDFFit format.

I think it would be better to also extract module and function names from the traceback. First, please update the example errortext in errorreportdialog.py to the traceback from https://groups.google.com/d/msg/diffpy-dev/H82rfFmokqM/wKHhsBBvz7wJ. For that traceback the search terms should be

errorwrapper.py
_f
mainframe.py
onSave
pdfguicontrol.py
save
UnicodeDecodeError
'ascii' codec can't decode byte 0xb0 in position 115: ordinal not in range(128)

resulting in the following search, which shows the pdfgui result first.
Note that searching for the last line only would show bug reports from other software.

PS: I suggest to use the regular expressions (module re) to extract file and function terms from a traceback text.

[pavoljuhas]

@dragonyanglong - how is it going? Any news or questions regarding this issue?

[dragonyanglong]

Hi @pavoljuhas , I have finished the work about extracting module and function names from the traceback using regular expression. I am now still working on incorporating your wx.html.HtmlWindow. Your prototype gist looks perfect to me. Sorry for the delay.

I have a quick question, how did you add wx.html.HtmlWindow using wxglade? I didn't find HtmlWindow in wxglade v.0.7.2 (or I miss any functions?) If I don't use wxglade to incorporate, I would worry about the sync between errorreportdialog.wxg and errorreportdialog.py.

[pavoljuhas]

@dragonyanglong - I did not use wxglade for the gist, it is a tweaked wx example.

wxglade indeed does not have HtmlWindow, but allows to insert a custom widget. It is the |?| button - just try to use it with HtmlWindow for the class name. See https://stackoverflow.com/questions/8941642/how-to-add-custom-widgets-in-wxglade for more details. It might be unavoidable to edit a line or two between the wxglade begin / end markers, but otherwise the wxg and py files should stay fairly in sync.

[dragonyanglong]

@pavoljuhas Thank you very much! Your custom widget method works well. Now, I think I have finished all the features required so far for this issue. How do you feel like the gui desigon of both modes?

In the error report mode, I feel like the current gap between the HtmlWindow and the Error log text is too wide. I tried different parameters wxGlade for the size of HtmlWindow, but it didn't work so far. It would look like this: I feel like it would look good if we can keep the Error Log text box size unchanged (show all traceback in one page instead of scroll down), and decrease the gap between the HtmlWindow and the Error log text. Do you know any parameters in wxGlade can help fix the problem?

I attached the wxGlade setting for HtmlWindow in below for clues.

Thank you!

[pavoljuhas]

I feel like it would look good if we can keep the Error Log text box size unchanged (show all traceback in one page instead of scroll down), and decrease the gap between the HtmlWindow and the Error log text.

Yes, good point, it would make it look much nicer. I think you need to add an extra sub-level of sizers as in the original design:

The 2 sub-sizers could be sizer_label and sizer_log to control the sizes of HtmlWindow and error log TextCtrl respectively. You can then set their relative proportions and also add some border around the label and log widgets. As it is they seem very close to the window border.

[dragonyanglong]

Thanks a lot @pavoljuhas . your method works perfect. I just submitted a PR. If any problems, please feel free to let me know and I can edit correspondingly. Thanks again for your time and help, I learned a lot from solving this issue.