jijo-paulose / ulipad

Automatically exported from code.google.com/p/ulipad
0 stars 0 forks source link

A big one #169

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Limodou, I made a relatively big patch. I hope you like it.

Original issue reported on code.google.com by vinet...@gmail.com on 5 Dec 2008 at 8:55

Attachments:

GoogleCodeExporter commented 9 years ago
There is one thing that needs to be corrected after you apply the patch. Please 
add 3
dots to the string 'Logging in' to be 'Logging in...' because this string 
indicates a
process and it should have 3 dots.

Let me show you how it should be...

Index: mixins/FtpClass.py
===================================================================
--- mixins/FtpClass.py  (revision 473)
+++ mixins/FtpClass.py  (working copy)
@@ -141,7 +141,7 @@
                     self.alive = False
                     self.running = False
                     return
-                common.setmessage(self.mainframe, tr('Loginning'))
+                common.setmessage(self.mainframe, tr('Logging in...'))
                 self.ftp.login(user, password)
             except socket.error, msg:
                 error.traceback()

Original comment by vinet...@gmail.com on 5 Dec 2008 at 9:09

GoogleCodeExporter commented 9 years ago
I think there are some problems about your patch:

1. You can see #161, the original caption is "Recent Files...", and you 
suggested me 
to change it to "Rencent Documents", and this time, you want me to change it 
back?

2. You want to change "Can't open the file [%s]." to "Can't open the file in 
the path 
%s.", I think it's not right, because it's indeed filename but not path.

3. You want to change

{{{
    if wx.Platform == '__WXMSW__':
        from modules import winreg
        for v in ('2.3', '2.4', '2.5', '2.6', '3.0'):
            try:
                key = winreg.Key(winreg.HKLM, 
r'SOFTWARE\Python\Pythoncore\%s\InstallPath' % v)
                interpreters.append((v+' console', os.path.join(key.value, 
'python.exe')))
                interpreters.append((v+' window', os.path.join(key.value, 
'pythonw.exe')))
            except:
                pass
}}}

to 
{{{
    if sys.platform == 'win32':
        from modules import winreg
        for v in ('2.3', '2.4', '2.5', '2.6', '3.0'):
            try:
                key = winreg.Key(winreg.HKLM, 
r'SOFTWARE\Python\Pythoncore\%s\InstallPath' % v)
                interpreters.append((v+' console', os.path.join(key.value, 
'python.exe')))
                interpreters.append((v+' window', os.path.join(key.value, 
'pythonw.exe')))
            except Exception:
                pass
}}}

I think they are almost the same, and I'll not change it.

4. "Uses tab as indent char or uses space as indent char." to "Uses a tab as an 
indent character or uses a space as indent character.", I think we don't need 
to 
indicate the number of character, and the number of space can be set in 
preference, 
and default is 4, but not one. So I'll change it to "Uses tabs as indent 
characters 
or uses spaces as indent characters.". And here the menu caption will be 
changed with 
the indent status changed.

5. "Command line of Open Command Window Here:" to "Path to the command-line 
interface 
of Open Command Window Here:", why I don't want to change it? Because command 
line is 
not just a path, but also including the parameters of it, so only path is not 
correct.

6. I think "The file" is better than "That file".

Original comment by limo...@gmail.com on 6 Dec 2008 at 3:33

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Let me reply to your comment by the same number you commented my patch.

####################
1. Yes, I have realized that I made a huge mistake thinking that those are 
actually
documents. But they are not. They are files. So please fix this as proposed in 
the
patch.

-- menu item: 'Recent Documents...' --> 'Recent Files...'

-- helpstring of this menu item: 'Shows the recently closed documents pop-up.' 
-->
'Shows recently opened files in a pop-up menu.'

An entry is added to that pop-up menu as soon as a file is opened, so there 
must be
the word 'opened'.

-- string in the preferences: 'Maximum number of recent documents:' --> 'Maximum
number of recent files:'

####################
2. You are right. But please remove the []. So please fix it like so:

-- "Can't open the file [%s]." --> "Can't open the file %s."

You really don't need the []. It works without them. I have tested. So please 
remove [].

####################
3. Let me explain:

-- if wx.Platform == '__WXMSW__'
This code checks if the graphical library is of Windows OS. The wx.Platform 
class is
a class of the wx graphical library, and all that the class Platform yields is 
which
graphical library is used on the OS you are on. There is no particular name (as 
far
as I know) of the Windows graphical library, so it's just MSW I guess. If you 
do  
import wx; wx.Platform   on a Linux machine, you would get  '__WXGTK__'. And 
this
means that on the Linux machine you are using the graphical library named GTK.

-- if sys.platform == 'win32'
This code checks if the OS is Windows. Actually it checks which API is used. 
And the
API of the Windows OS is named Win32. Under Linux, doing   import wx; 
sys.platform  
would yield 'linux2'.

In the case of that particular part of UliPad's source code, I suggest to test 
which
OS is used rahter than which graphical library is used because the user can run 
the
GTK graphical library on Windows, so your code would fail in that case. Better 
to
check which OS is used and then execute code by that. So I suggest  if 
sys.platform
== 'win32'.

Okay, now for the 'except vs. except Exception'. If you use 'except', the user 
can
crash UliPad with pressing Ctrl+C when that code is executing, because the 
result is
a KeyboardInterrupt event which you didn't catch. There is a simple solution: 
use 
'except Exception'! The 'Exception' is the base class of all exceptions in 
Python.
It's generally not a good idea to catch all exceptions because a lot of them 
can't
even be triggered. But as I was looking at that particular part of the source 
code, I
also didn't know which exceptions can occur in that case, so the only way is to 
check
for all exceptions. I suggest that you use 'except Exception' everytime you 
want to
catch all exceptions. There is also another exception not being caught by 
'except'
and that is SystemExit, but I won't go into the details for that event. Please 
use
'except Exception' everywhere in the source code you are catching all 
exceptions.

####################
4. Yes, your version of the help string is better than mine. Please fix it as 
you
proposed. But have in mind that this menu item is toggling, so you need to make 
this
correctly by fixing two helpstrings.

####################
5. Yes, you are right. I think that the current string is okay after all.

####################
6. Yes, it is better. I agree.

###################################
You forgot to fix one helpstring which needs a dot at the end.

'Toggles the word wrap feature of the current document' --> 'Toggles the word 
wrap
feature of the current document.'

Limodou, you need to trust me more. If I fix something, I have put a lot of 
thought
into the fix. Next time, please trust me that my version is good. I don't 
always have
the time to explain why my version is better. I really love UliPad and I want 
that it
becomes perfect. I have a lot of patches to give you because there are a lot of
things to be fixed, especially strings. I know that it's difficult for you to
correctly express yourself in the English language because your native language 
is
Chinese, and between those two languages is a huge difference. So let me help 
you
correct your expressions. I must admit that English is not my native language as
well, but I am a professional English speaker. I don't want to brag myself but I
think I am good at this so I can help. I am a human so I make mistakes. Please
forgive me for that. But as time goes by, I re-think about the fixes I proposed 
and I
sometimes realize that I was wrong; then I propose that some string (or 
whatever) be
fixed back as it was, just with a little change to be made to the original 
version of
the string (or whatever).

I must say that I am very precise. I can even say that you won't find any 
mistake in
the text of this reply to your comment. I am editing it so long that it's 
perfect.
The same goes for UliPad's strings (or whatever I propose).

Oh, I almost forgot. I don't know if you noticed but I am Chester Dunn. :)  
Well,
that's not my real name but I am the same person. So I am asking if you can 
remove
the author 'Chester Dunn' under 'Also thanks a lot' from the AUTHORS.txt file,
because you already have 'Vinetouu' in that authors list under 'Great thanks 
to'. Why
have two same authors? :)  My real name is Boštjan Mejak. You can copy & paste 
my
real name into the authors list and replace 'Vinetouu' with 'Boštjan Mejak'. 
The 3rd
letter of my name is a non-ASCII character, so please make the file AUTHORS.txt 
use
the encoding UTF-8 (if it doesn't use it already), otherwise the 3rd letter of 
my
name won't be correctly displayed there.

Okay, so this is everything I wanted to say. Be good and have fun! ;)  I have 
some
stuff to do, so I'll provide new patches some other day. Please fix all the 
things
mentioned in this reply. That would certainly improve UliPad.

Original comment by vinet...@gmail.com on 6 Dec 2008 at 5:45

GoogleCodeExporter commented 9 years ago
For 3, I think wx.Platform just represents the different platform, wxpython 
uses 
native library, so you will not get '__WXGTK__' on windows platform for now, so 
I 
think it's safe enough. And for ctrl+C will stop the ulipad, how can you do 
that? I 
think only when you start ulipad with UliPad.py and press Ctrl+C in terminal 
window, 
and I think it's a good thing, if ulipad has some problems, you can easy stop 
it. So 
that's what I want.

For 4, I don't want to spend much time on it, because I should change the menu 
caption when the status changed, it need some work to do.

You've done many things for ulipad, and I'm very appreciate you and your work. 

But you may know that I'm not a prefect person, I like simple is better and 
enough is 
better, so many things I don't want to change to much especially on some 
details, 
because little people care about them including me, but you is a exception. And 
I 
think the functionality of help string or menu caption are little, if you want, 
you 
can just like pyogrammer.py, writing some documents for ulipad to help people 
to 
understand it and use it better. You can see the site 
http://simiansector.com/ulipad-
notes/index.html, it's very good I think.

And thank you again, hope you continuely support ulipad.

Original comment by limo...@gmail.com on 7 Dec 2008 at 7:42

GoogleCodeExporter commented 9 years ago
Please recreate the mixins/Import.py file, because UliPad still has the old 
strings
in Import.py module and so the new strings are not displayed.

Original comment by vinet...@gmail.com on 7 Dec 2008 at 12:33

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago

Original comment by limo...@gmail.com on 15 Dec 2008 at 1:48