Multiple issues · Pull Request #84 · bruderstein/PythonScript

ghost · 2018-06-27T11:24:34Z

implements feature request #68, #76 and fixes #83

separate outputs with an empty line (feature request #68) partially.

Yaron10 · 2018-06-27T13:47:47Z

Hello Claudia,

Thank you for your beautiful work. Highly appreciated.
It's actually better than what I had expected. 👍

Allow me some comments:

Syntax Error message after Console -> Run (or Ready.) is not separated by an empty line.
Console -> Run after Syntax Error message is separated by two empty lines.

Have you decided not to flash the Python title bar?
Looking at NPP code it seems rather simple.

void FindReplaceDlg::setStatusbarMessage(const generic_string & msg, FindStatus staus)
{
	if (staus == FSNotFound)
	{
		::MessageBeep(0xFFFFFFFF);

		FLASHWINFO flashInfo;
		flashInfo.cbSize = sizeof(FLASHWINFO);
		flashInfo.hwnd = isVisible()?_hSelf:GetParent(_hSelf);   // Relevant line to Python.
		flashInfo.uCount = 3;
		flashInfo.dwTimeout = 100;
		flashInfo.dwFlags = FLASHW_ALL;
		FlashWindowEx(&flashInfo);
	}

The Automatically open console on error setting is applied immediately.
The other two new settings require a restart.
If they can not be applied immediately, I think requires restart should be added.

And some minor comments:

How about placing the Automatically open console on error setting first?
The other two are more closely related.
👍 for adding ... to Choose a color. :)
Can you add it to Set Icon as well?
With your new settings, the Configuration Dlg title should certainly be changed to Python Script Configuration.

Best regards.

ghost · 2018-06-27T20:13:58Z

Hi Yaron - I'm a little bit confused as I thought I have already answered concerning the flash window issue but, just checked it and it isn't added in the issue. My understanding is that only toplevel windows
can be flashed.

Concerning adding an additional eol, the prompt is used to reflect this on a normal run and therefore it can be added only as \n>>> otherwise the next statement is on the next line but to have exceptions also
separated it is needed to check return code from PyRun_SimpleString but in that case the exception has been already written and it can be only added to the end - so this situation can happen, yes.

All other settings do require a restart as well, maybe Automatically open console on error should be changed to reflect the others.

What do you mean by setting it to Set icon ? Put it next to the Set icon button? No, or? It doesn't
has anything to do with this section.

Placing Automatically open console on error to the top of the new three checkbuttons, yes, why not,
same is true for renaming title to Python Script Configuration

Yaron10 · 2018-06-27T20:46:25Z

Hello Claudia,

My understanding is that only toplevel windows
can be flashed.

Thank you for looking into it.
How about using ::MessageBeep(0xFFFFFFFF); on error?

Concerning adding an additional eol, the prompt is used to reflect this on a normal run
...

So both

Syntax Error message after Console -> Run (or Ready.) is not separated by an empty line.
Console -> Run after Syntax Error message is separated by two empty lines.

can't be changed, is that correct?

All other settings do require a restart as well, maybe Automatically open console on error should be changed to reflect the others.

How about adding (applied immediately) to Automatically open console on error?

What do you mean by setting it to Set icon?

I meant changing Set Icon to Set Icon... while you're at it.

Placing Automatically open console on error to the top of the new three checkbuttons, yes, why not,
same is true for renaming title to Python Script Configuration

👍

Great work. Thanks again.

ghost · 2018-06-27T21:07:24Z

If the exception is generated by the script it behaves different as to when it is generated via Run dialog

Via script execution it is needed to add it to the end of the exception text which then can lead to your reported behavior - I don't see, at the moment, how this could be solved easily. I mean without interacting with the boost::python library which scars me :-)

In which cases should it beep - only if error is generated via script execution or also if exception is generated via run dialog.

Changes to ordering and Set icon can be done - hopefully @chcg doesn't start 1.2 build right now

chcg · 2018-06-27T21:08:56Z

@ClaudiaFrank No I didn't start yet. Will wait for your go :-)

Yaron10 · 2018-06-27T21:35:39Z

I don't see, at the moment, how this could be solved easily. I mean without interacting with the boost::python library which scars me :-)

Great work as it is.
“The Only Thing We Have to Fear Is Fear Itself.”
I'm joking; I can certainly understand you.

In which cases should it beep - only if error is generated via script execution or also if exception is generated via run dialog.

If "Python" is hidden there's no need to beep; showing "Python" is a good alert.
In Scott's case (i.e. open but some other element (e.g. Find Results) is actually visible), I'd beep.
I think there's no need to beep if exception is generated via run dialog.

How about adding (applied immediately) to Automatically open console on error?

Actually, (no restart) might be better as you need to press OK.

Changes to ordering and Set icon can be done

👍

Thanks again.

Yaron10 · 2018-06-27T22:11:49Z

If "Python" is hidden there's no need to beep; showing "Python" is a good alert.

It might be better not to complicate things and always beep on script error.
What do you think?

ghost · 2018-06-27T22:12:04Z

Hi Yaron,

“The Only Thing We Have to Fear Is Fear Itself.”
absolutely true :-)

If "Python" is hidden there's no need to beep; showing "Python" is a good alert.
In Scott's case (i.e. open but some other element (e.g. Find Results) is actually visible), I'd beep.
I think there's no need to beep if exception is generated via run dialog.

Isn't this the current (with the recent changes) behavior?
I've tested with function list window, python script console and find in files result window.
Regardless which one had the focus, as soon as an exception triggers an show console action it has the
focus - it is the same as if it was hidden, isn't it? Do I misunderstand something?

ghost · 2018-06-27T22:15:22Z

I'm unsure if beeping on run dialog error is easy to implement - currently unsure if it is needed at all
taking into account that the console is "somehow flashing :-)" in case of error

ghost · 2018-06-27T22:18:28Z

what about (no restart required) instead?
Implies that the others do need a restart to take into effect.

Yaron10 · 2018-06-27T22:22:12Z

Isn't this the current (with the recent changes) behavior?
I've tested with function list window, python script console and find in files result window.
Regardless which one had the focus, as soon as an exception triggers an show console action it has the
focus - it is the same as if it was hidden, isn't it? Do I misunderstand something?

Your implementation is perfect.
I was referring to beeping.

I'm unsure if beeping on run dialog error is easy to implement - currently unsure if it is needed at all
taking into account that the console is "somehow flashing :-)" in case of error

No beeping then. :)
Again, great work as it is.

what about (no restart required) instead?

Better. :)

Thank you. I appreciate your work.

ghost · 2018-06-27T22:33:38Z

Ok, thx for all the testing, inspirations and ideas.
I will do the changes and do the commit so that @chcg can do the merge and start the build :-)

Yaron10 · 2018-06-27T22:36:55Z

Great.
Thanks again for your hard work and beautiful implementation.

Good night.

ghost · 2018-06-27T23:06:52Z

@chcg GOOO :-D

Yaron10 · 2018-06-27T23:18:18Z

VERY GOOD. :)

Just add some width to the second setting.

ghost · 2018-06-28T10:48:57Z

Yours look very different to mine - not only the theme but it looks like the button isn't centered as well.

Is this ReactOS or Windows XP?

Yaron10 · 2018-06-28T13:02:00Z

Hello Claudia,

It's Windows 7 - Classic Look Theme.
The Cancel button is 1px higher then OK (in yours too). :)
And could you please re-commit @chcg's 5969bb8?

Thank you.

chcg · 2018-06-28T16:56:43Z

@Yaron10 @ClaudiaFrank

I will have a look at that. Shouldn't it be choose https://dict.leo.org/englisch-deutsch/choose, instead of "chose a color..."

Done with c735574

Yaron10 · 2018-06-28T18:03:51Z

Hello @chcg,

Shouldn't it be choose https://dict.leo.org/englisch-deutsch/choose, instead of "chose a color..."

👍

I will have a look at that.

Thank you. I appreciate that.

Yaron10 · 2018-06-28T23:31:55Z

Run notepad.getCurrentView from the console.

Result:

I don't understand the message but it's certainly an error.
Can it be colorized in red?

Thank you.

ghost · 2018-06-29T00:34:03Z

No it is not an error it is the information that getCurrentView is a method of the class Notepad which
is bound to the object Npp.Notepad at address 0x03FFA5B0.

sasumner · 2018-06-29T11:39:34Z

@Yaron10 Really obvious now I'm sure but to run it you need to do notepad.getCurrentView(), not notepad.getCurrentView. :-)

ghost · 2018-06-29T11:54:11Z

Just another hint why this can't be treated as an error at all - imagine you want to provide a method
as a parameter to another function like in editor.forEachLine().
This wouldn't be possible if python wouldn't treat it like it is.

Yaron10 · 2018-06-29T15:25:00Z

Is notepad.getCurrentView a legit Run command?
Let me refine the question: if there was a list of legit Run commands, would notepad.getCurrentView (or any other function without ()) be included in it?

Just another hint why this can't be treated as an error at all - imagine you want to provide a method
as a parameter to another function like in editor.forEachLine().
This wouldn't be possible if python wouldn't treat it like it is.

👍
I suppose that settles my question.

@sasumner,

but to run it you need to do notepad.getCurrentView(), not notepad.getCurrentView. :-)

I certainly was aware of that. I just thought notepad.getCurrentView was an error and therefore the output should be colorized accordingly.

ghost · 2018-06-29T15:48:47Z

Is notepad.getCurrentView a legit Run command?

Yes

Is notepad.getCurrentView( a legit Run command?

Yes again.

Python allows you to define function/methods on one line and parameters on a different line.

Yaron10 · 2018-06-29T20:28:04Z

👍

Thank you Claudia.

CFrank added 7 commits June 18, 2018 00:21

implements display console on error (feature request #76) and

ce3d15d

separate outputs with an empty line (feature request #68) partially.

config file and configuration related changes

40bfaf2

implement colorchooser

cf0f849

put stoi in try catch block to avoid non integer conversion exception

b459f5a

Documentation updates

6928ddf

fix some missing return tags in notepad.rst

d8e4afa

fixes issue #81

b227cba

chcg added this to the v1.2 milestone Jun 27, 2018

gui changes as discussed in #84

3806ff9

chcg merged commit e1e3621 into bruderstein:master Jun 27, 2018

Conversation

ghost commented Jun 27, 2018

Uh oh!

Yaron10 commented Jun 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Jun 27, 2018 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Yaron10 commented Jun 27, 2018

Uh oh!

ghost commented Jun 27, 2018

Uh oh!

chcg commented Jun 27, 2018

Uh oh!

Yaron10 commented Jun 27, 2018

Uh oh!

Yaron10 commented Jun 27, 2018

Uh oh!

ghost commented Jun 27, 2018

Uh oh!

ghost commented Jun 27, 2018

Uh oh!

ghost commented Jun 27, 2018 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Yaron10 commented Jun 27, 2018

Uh oh!

ghost commented Jun 27, 2018

Uh oh!

Yaron10 commented Jun 27, 2018

Uh oh!

ghost commented Jun 27, 2018

Uh oh!

Yaron10 commented Jun 27, 2018

Uh oh!

ghost commented Jun 28, 2018

Uh oh!

Yaron10 commented Jun 28, 2018

Uh oh!

chcg commented Jun 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Yaron10 commented Jun 28, 2018

Uh oh!

Yaron10 commented Jun 28, 2018

Uh oh!

ghost commented Jun 29, 2018

Uh oh!

sasumner commented Jun 29, 2018

Uh oh!

ghost commented Jun 29, 2018

Uh oh!

Yaron10 commented Jun 29, 2018

Uh oh!

ghost commented Jun 29, 2018

Uh oh!

Yaron10 commented Jun 29, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Yaron10 commented Jun 27, 2018 •

edited

Loading

ghost commented Jun 27, 2018 •

edited by ghost

Loading

ghost commented Jun 27, 2018 •

edited by ghost

Loading

chcg commented Jun 28, 2018 •

edited

Loading