Below code produces a basic text editor that I have written using tkinter with python. I tried to be as well structured as I can but in some parts I have been lazy about it.
Classes that are used:
MainWindow
: I tried to manage anything topmost level using this class but I was unable to tell if something was 'topmost level' or not at some parts such as changing the title using a children's method likenew_file
.MainFrame
: This class was really the non-configuration parts of the 'topmost level' stuff, and was created with concerns of putting it in anotherToplevel
widget. It holds the information on widget's configurations and their methods. But again, at the end I don't really see a strict line between this andMainWindow
.MainMenu
: This class is the structure class to hold sub-menus.FileMenu
: This is a sub-menu class to be assigned as file related parts of theMainMenu
.EditMenu
: This is a sub-menu class to be assigned as edit related parts of theMainMenu
.AutoScrollbar
: This exists for smart scrollbar visibility.
Here's the code:
#required for anything GUI related
import tkinter as tk
#required for fileopen and save options
from tkinter import filedialog
#required for file's basename
import os
#required for save_before_leave basically
from tkinter import messagebox
#Nae Unicode text entering is missing
class MainWindow(tk.Tk):
def __init__(self):
super().__init__()
self.title("Untitled - NaePad")
self.mainFrame = MainFrame(self)
self.mainFrame.pack(fill="both", expand=True)
self.mainFrame.text.focus_set()
self.protocol("WM_DELETE_WINDOW", lambda : self.save_before_leave(self.destroy))
def save_before_leave(self, callback, *args):
textBuffer = self.mainFrame.text.get('1.0', 'end-1c')
#if there's a current file
if self.mainFrame.curFilePath:
#if it content doesn't match the current buffer
if self.mainFrame.curFileCont != textBuffer:
response = messagebox.askyesnocancel("NaePad - Unsaved File", "Do you want to save before leaving?")
#if response is yes
if response:
self.mainFrame.save_file()
callback()
#if response is no
elif response is False:
callback()
else:
callback()
else:
#if there's any text in the buffer
if textBuffer:
response = messagebox.askyesnocancel("NaePad - Unsaved File", "Do you want to save before leaving?")
#If it's a yes
if response:
self.mainFrame.save_as_file()
callback()
#if it's a no
elif response is False:
callback()
else:
callback()
#required in order to prevent "tagbinds" from happening
return "break"
class MainFrame(tk.Frame):
def __init__(self, master):
super().__init__(master)
self.master = master
#creates and displays the Menu
self.menu = MainMenu(master)
master.config(menu=self.menu)
#path of the current file
self.curFilePath = ''
#current file
self.curFileCont = ''
self.text = tk.Text(self, wrap="none")
#color scheme
self.text.config(bg='#282c34',fg='#abb2bf', selectbackground='#3e4451')
self.text.grid(row=0, column=0, sticky="nsew")
self.rowconfigure(0, weight=1)
self.columnconfigure(0, weight=1)
self.scroll_bar_config()
self.menu_command_config()
self.key_binds_config()
#scrollbar configurations for self.text widget
def scroll_bar_config(self):
self.scrollY = AutoScrollbar(self, orient="vertical", command=self.text.yview)
self.scrollY.grid(row=0, column=1, sticky="nsew")
self.text['yscrollcommand'] = self.scrollY.set
self.scrollX = AutoScrollbar(self, orient="horizontal", command=self.text.xview)
self.scrollX.grid(row=1, column=0, sticky="nsew")
self.text['xscrollcommand'] = self.scrollX.set
def menu_command_config(self):
#New
self.menu.file.entryconfig(0, command=lambda : self.master.save_before_leave(self.new_file))
#Open...
self.menu.file.entryconfig(1, command=lambda : self.master.save_before_leave(self.open_file))
#Save
self.menu.file.entryconfig(2, command=self.save_file)
#Save as
self.menu.file.entryconfig(3, command=self.save_as_file)
#Cut
self.menu.edit.entryconfig(0, command=self.cut)
#Copy
self.menu.edit.entryconfig(1, command=self.copy)
#Paste
self.menu.edit.entryconfig(2, command=self.paste)
#Delete
self.menu.edit.entryconfig(3, command=self.delete)
def key_binds_config(self):
self.text.bind('<Control-n>', lambda event : self.master.save_before_leave(self.new_file))
self.text.bind('<Control-N>', lambda event : self.master.save_before_leave(self.new_file))
self.text.bind('<Control-o>', lambda event : self.master.save_before_leave(self.open_file))
self.text.bind('<Control-O>', lambda event : self.master.save_before_leave(self.open_file))
self.text.bind('<Control-s>', self.save_file)
self.text.bind('<Control-S>', self.save_file)
self.text.bind('<Control-Shift-s>', self.save_as_file)
self.text.bind('<Control-Shift-S>', self.save_as_file)
def new_file(self, *args):
self.text.delete("1.0", "end")
self.curFilePath = ''
self.curFileCont = ''
#update window name to file name BAD NAE BAD
self.master.title("Untitled - NaePad")
def open_file(self, *args):
#get filepath from user with gui
filePath = filedialog.askopenfilename(filetypes=(("Text files", "*.txt"),("All files", "*.*")))
# if filePath is selected
if filePath:
try:
#open only UTF-8 encoded files
with open(filePath, encoding="UTF-8") as f:
self.text.delete("1.0", "end")
self.curFileCont = f.read()
self.text.insert("1.0", self.curFileCont)
#if it's not UTF-8 then
except UnicodeDecodeError:
#open as ANSI encoding
with open(filePath, encoding="ANSI") as f:
self.text.delete("1.0", "end")
self.curFileCont = f.read()
self.text.insert("1.0", self.curFileCont)
#update window name to file name BAD NAE BAD
self.master.title(os.path.basename(f.name) + " - NaePad")
#update current file path
self.curFilePath = filePath
def save_file(self, *args):
#if there's already a file
if self.curFilePath:
try:
#open only UTF-8 encoded files
with open(self.curFilePath, 'w', encoding="UTF-8") as f:
self.curFileCont = self.text.get('1.0', 'end-1c')
f.write(self.curFileCont)
#if it's not UTF-8 then
except UnicodeDecodeError:
#open as ANSI encoding
with open(self.curFilePath, 'w', encoding="ANSI") as f:
self.curFileCont = self.text.get('1.0', 'end-1c')
f.write(self.curFileCont)
#update window name to file name BAD NAE BAD
self.master.title(os.path.basename(f.name) + " - NaePad")
else:
self.save_as_file()
def save_as_file(self, *args):
self.curFilePath = filedialog.asksaveasfilename(defaultextension=".txt", filetypes=(("Text files", "*.txt"),("All files", "*.*")))
#Checks wether a file is selected or not
if self.curFilePath:
self.save_file()
def cut(self):
#Is anything selected?
if self.text.tag_ranges(tk.SEL):
self.text.clipboard_clear()
#append to cleared clipboard the (selection)
self.text.clipboard_append(self.text.get(tk.SEL_FIRST, tk. SEL_LAST))
self.text.delete(tk.SEL_FIRST, tk. SEL_LAST)
def copy(self):
#Is anything selected?
if self.text.tag_ranges(tk.SEL):
self.text.clipboard_clear()
#append to cleared clipboard the (selection)
self.text.clipboard_append(self.text.get(tk.SEL_FIRST, tk. SEL_LAST))
def paste(self):
#Is anything selected?
if self.text.tag_ranges(tk.SEL):
#keeping a reference on where the selection starts
selFirstIndex = self.text.index(tk.SEL_FIRST)
#removing selection first
self.text.delete(tk.SEL_FIRST, tk. SEL_LAST)
#copying from clipboard
self.text.insert(selFirstIndex, self.text.clipboard_get())
else:
self.text.insert(tk.INSERT, self.text.clipboard_get())
def delete(self):
#Is anything selected?
if self.text.tag_ranges(tk.SEL):
self.text.delete(tk.SEL_FIRST, tk. SEL_LAST)
#MainMenu object that contains Sub-menus
class MainMenu(tk.Menu):
def __init__(self, master):
super().__init__(master, tearoff=0)
self.master = master
#create Menu Options
self.file = FileMenu(self)
self.add_cascade(label="File", menu=self.file)
self.edit = EditMenu(self)
self.add_cascade(label="Edit", menu=self.edit)
#Menu class that handles File Operations
class FileMenu(tk.Menu):
def __init__(self, master):
super().__init__(master, tearoff=0)
self.master = master
self.add_command(label="New Ctrl + N")
self.add_command(label="Open... Ctrl + O")
self.add_command(label="Save Ctrl + S")
self.add_command(label="Save As... Ctrl + Shift + S")
#add the line before exit
self.add_separator()
#destroy's the grandparent, which is assumed to be a toplevel BAD NAE BAD
self.add_command(label="Exit Alt + F4", command=lambda : master.master.save_before_leave(self.master.master.destroy))
#Menu class that handles editorial operations
class EditMenu(tk.Menu):
def __init__(self, master):
super().__init__(master, tearoff=0)
self.add_command(label="Cut Ctrl + X")
self.add_command(label="Copy Ctrl + C")
self.add_command(label="Paste Ctrl + V")
self.add_command(label="Delete Delete")
#http://effbot.org/zone/tkinter-autoscrollbar.htm
class AutoScrollbar(tk.Scrollbar):
# a scrollbar that hides itself if it's not needed. only
# works if you use the grid geometry manager.
def set(self, lo, hi):
if float(lo) <= 0.0 and float(hi) >= 1.0:
self.grid_remove()
else:
self.grid()
tk.Scrollbar.set(self, lo, hi)
def test():
with open(__file__, "rU") as f:
root.mainFrame.text.insert("1.0", f.read())
if __name__ == "__main__":
root = MainWindow()
#test()
root.mainloop()
Review Concern(s):
- My main concern is to code in a, easy to read, efficient, and well structured manner while still learning the language and concepts such as OOP. Feel free to mention tiniest issue or improvement that comes to your mind, as I am a beginner and I probably need it.
My own critique:
There are some inconsistencies between arguments to similar methods such as insert("1.0",...)
and insert('1.0', ...)
I want to structure my ""
and ''
usage so that I can better write and understand code but I am undecided which one to use in some situations.
I question the very necessity of MainFrame
class, I think I should have removed it and assigned its children directly to MainWindow
class instead.
Using self.master.something
feels pretty wrong let alone using master.master.something
but I am not entirely sure on what to do when there's a method that needs access to both a parent and a grandchildren's parameters, I would assume that's a parent's method than a children's.
I am most proud of the save_file
and save_as_file
methods as I believe they co-depend on each other very well. Still there's the issue of their level though.
There's the issue of encodings, which turned out to be a much harder problem than I initially anticipated. I had to use try
, except
for an easy way out.
-
1\$\begingroup\$ You missed a line in the Rollback. Did you know there's an actual 'rollback' button in every revision of your question except the last? \$\endgroup\$Mast– Mast ♦2017年12月04日 15:05:50 +00:00Commented Dec 4, 2017 at 15:05
-
\$\begingroup\$ @Mast Thanks for the help but I've read it's available if you have full edit privilege. Is it not? Though according to this the right thing to do is to flag as other and ask for a rollback. \$\endgroup\$Nae– Nae2017年12月04日 15:42:03 +00:00Commented Dec 4, 2017 at 15:42
-
1\$\begingroup\$ I thought there was an exception for questions you own, but you could well be correct. Anyway, we got it nicely sorted out now. Feel free to find us in chat if you ever need help :-) \$\endgroup\$Mast– Mast ♦2017年12月04日 16:46:30 +00:00Commented Dec 4, 2017 at 16:46
2 Answers 2
Few things I would bring your attention to:
- naming. It is recommended by the PEP8 styleguide and generally agreed to follow the
lower_case_with_underscores
variable and function naming style. You have violated it on multiple occasions - e.g.mainFrame
,curFilePath
,curFileCont
etc. - comments are over-used. Code without comments might feel like an extreme and somewhat opinion-based, but it is generally a great way of thinking about self-documented code. Just to highlight a few spots - I don't see the usefulness of
#path of the current file
before theself.curFilePath = ''
line - especially if you would name a variablecurrent_file_path
. And, you can and should convert some of the comments into proper Documentation Strings - code repetition. Don't repeat yourself. Think of extracting a method out of the
save_before_leave()
method - look how the bodies of the twoif
blocks are identical to each other
-
1\$\begingroup\$ Based on your last point I noticed that they can be identical(initially they called different save methods) and then written a new method
response_handle
for that part, and updated my code. Will work on other points as well. Thanks a lot for the review. \$\endgroup\$Nae– Nae2017年12月04日 14:44:38 +00:00Commented Dec 4, 2017 at 14:44 -
3\$\begingroup\$ @Nae Please see this Meta post, you might want to rollback your last edit codereview.meta.stackexchange.com/questions/1763/… \$\endgroup\$Ludisposed– Ludisposed2017年12月04日 14:52:43 +00:00Commented Dec 4, 2017 at 14:52
#required for anything GUI related
import tkinter as tk
#required for fileopen and save options
from tkinter import filedialog
#required for file's basename
import os
#required for save_before_leave basically
from tkinter import messagebox
#Nae Unicode text entering is missing
class MainWindow(tk.Tk):
def __init__(self):
super().__init__()
self.title("Untitled - NaePad")
self.mainFrame = MainFrame(self)
self.mainFrame.pack(fill="both", expand=True)
self.mainFrame.text.focus_set()
self.protocol("WM_DELETE_WINDOW", lambda : self.save_before_leave(self.destroy))
def save_before_leave(self, callback, *args):
textBuffer = self.mainFrame.text.get('1.0', 'end-1c')
#if there's a current file
if self.mainFrame.curFilePath:
#if it content doesn't match the current buffer
if self.mainFrame.curFileCont != textBuffer:
self.response_handle(callback)
else:
callback()
else:
#if there's any text in the buffer
if textBuffer:
self.response_handle(callback)
else:
callback()
#required in order to prevent "tagbinds" from happening
return "break"
def response_handle(self, callback):
response = messagebox.askyesnocancel("NaePad - Unsaved File", "Do you want to save before leaving?")
#if response is yes
if response:
self.mainFrame.save_file()
callback()
#if response is no
elif response is False:
callback()
class MainFrame(tk.Frame):
def __init__(self, master):
super().__init__(master)
self.master = master
#creates and displays the Menu
self.menu = MainMenu(master)
master.config(menu=self.menu)
#path of the current file
self.curFilePath = ''
#current file
self.curFileCont = ''
self.text = tk.Text(self, wrap="none")
#color scheme
self.text.config(bg='#282c34',fg='#abb2bf', selectbackground='#3e4451')
self.text.grid(row=0, column=0, sticky="nsew")
self.rowconfigure(0, weight=1)
self.columnconfigure(0, weight=1)
self.scroll_bar_config()
self.menu_command_config()
self.key_binds_config()
#scrollbar configurations for self.text widget
def scroll_bar_config(self):
self.scrollY = AutoScrollbar(self, orient="vertical", command=self.text.yview)
self.scrollY.grid(row=0, column=1, sticky="nsew")
self.text['yscrollcommand'] = self.scrollY.set
self.scrollX = AutoScrollbar(self, orient="horizontal", command=self.text.xview)
self.scrollX.grid(row=1, column=0, sticky="nsew")
self.text['xscrollcommand'] = self.scrollX.set
def menu_command_config(self):
#New
self.menu.file.entryconfig(0, command=lambda : self.master.save_before_leave(self.new_file))
#Open...
self.menu.file.entryconfig(1, command=lambda : self.master.save_before_leave(self.open_file))
#Save
self.menu.file.entryconfig(2, command=self.save_file)
#Save as
self.menu.file.entryconfig(3, command=self.save_as_file)
#Cut
self.menu.edit.entryconfig(0, command=self.cut)
#Copy
self.menu.edit.entryconfig(1, command=self.copy)
#Paste
self.menu.edit.entryconfig(2, command=self.paste)
#Delete
self.menu.edit.entryconfig(3, command=self.delete)
def key_binds_config(self):
self.text.bind('<Control-n>', lambda event : self.master.save_before_leave(self.new_file))
self.text.bind('<Control-N>', lambda event : self.master.save_before_leave(self.new_file))
self.text.bind('<Control-o>', lambda event : self.master.save_before_leave(self.open_file))
self.text.bind('<Control-O>', lambda event : self.master.save_before_leave(self.open_file))
self.text.bind('<Control-s>', self.save_file)
self.text.bind('<Control-S>', self.save_file)
self.text.bind('<Control-Shift-s>', self.save_as_file)
self.text.bind('<Control-Shift-S>', self.save_as_file)
#method handling file operations with encoding in mind
def file_operation(self, file_path, file_opt='r'):
#if file_path exists
if file_path:
try:
#open only UTF-8 encoded files
with open(file_path, file_opt, encoding="UTF-8") as f:
self.data_operation(f, file_opt)
#if it's not UTF-8 then
except UnicodeDecodeError:
#open as ANSI encoding
with open(file_path, fil_opt, encoding="ANSI") as f:
self.data_operation(f, file_opt)
#update current file path
self.curFilePath = file_path
#update window name to file name BAD NAE BAD
self.master.title(os.path.basename(f.name) + " - NaePad")
#If it's a save operation and there's no file_path
elif ((not file_path) and (file_opt == 'w')):
self.save_as_file()
#data writing and reading method
def data_operation(self, file_data, file_opt):
#read data
if file_opt == 'r':
self.text.delete("1.0", "end")
self.curFileCont = file_data.read()
self.text.insert("1.0", self.curFileCont)
#write data
elif file_opt == 'w':
self.curFileCont = self.text.get('1.0', 'end-1c')
file_data.write(self.curFileCont)
def new_file(self, *args):
self.text.delete("1.0", "end")
self.curFilePath = ''
self.curFileCont = ''
#update window name to file name BAD NAE BAD
self.master.title("Untitled - NaePad")
def open_file(self, *args):
#get filepath from user with gui
filePath = filedialog.askopenfilename(filetypes=(("Text files", "*.txt"),("All files", "*.*")))
self.file_operation(filePath, 'r')
def save_file(self, *args):
self.file_operation(self.curFilePath, 'w')
def save_as_file(self, *args):
self.curFilePath = filedialog.asksaveasfilename(defaultextension=".txt", filetypes=(("Text files", "*.txt"),("All files", "*.*")))
#Checks wether a file is selected or not
if self.curFilePath:
self.save_file()
def cut(self):
#Is anything selected?
if self.text.tag_ranges(tk.SEL):
self.text.clipboard_clear()
#append to cleared clipboard the (selection)
self.text.clipboard_append(self.text.get(tk.SEL_FIRST, tk. SEL_LAST))
self.text.delete(tk.SEL_FIRST, tk. SEL_LAST)
def copy(self):
#Is anything selected?
if self.text.tag_ranges(tk.SEL):
self.text.clipboard_clear()
#append to cleared clipboard the (selection)
self.text.clipboard_append(self.text.get(tk.SEL_FIRST, tk. SEL_LAST))
def paste(self):
#Is anything selected?
if self.text.tag_ranges(tk.SEL):
#keeping a reference on where the selection starts
selFirstIndex = self.text.index(tk.SEL_FIRST)
#removing selection first
self.text.delete(tk.SEL_FIRST, tk. SEL_LAST)
#copying from clipboard
self.text.insert(selFirstIndex, self.text.clipboard_get())
else:
self.text.insert(tk.INSERT, self.text.clipboard_get())
def delete(self):
#Is anything selected?
if self.text.tag_ranges(tk.SEL):
self.text.delete(tk.SEL_FIRST, tk. SEL_LAST)
#MainMenu object that contains Sub-menus
class MainMenu(tk.Menu):
def __init__(self, master):
super().__init__(master, tearoff=0)
self.master = master
#create Menu Options
self.file = FileMenu(self)
self.add_cascade(label="File", menu=self.file)
self.edit = EditMenu(self)
self.add_cascade(label="Edit", menu=self.edit)
#Menu class that handles File Operations
class FileMenu(tk.Menu):
def __init__(self, master):
super().__init__(master, tearoff=0)
self.master = master
self.add_command(label="New Ctrl + N")
self.add_command(label="Open... Ctrl + O")
self.add_command(label="Save Ctrl + S")
self.add_command(label="Save As... Ctrl + Shift + S")
#add the line before exit
self.add_separator()
#destroy's the grandparent, which is assumed to be a toplevel BAD NAE BAD
self.add_command(label="Exit Alt + F4", command=lambda : master.master.save_before_leave(self.master.master.destroy))
#Menu class that handles editorial operations
class EditMenu(tk.Menu):
def __init__(self, master):
super().__init__(master, tearoff=0)
self.add_command(label="Cut Ctrl + X")
self.add_command(label="Copy Ctrl + C")
self.add_command(label="Paste Ctrl + V")
self.add_command(label="Delete Delete")
#http://effbot.org/zone/tkinter-autoscrollbar.htm
class AutoScrollbar(tk.Scrollbar):
# a scrollbar that hides itself if it's not needed. only
# works if you use the grid geometry manager.
def set(self, lo, hi):
if float(lo) <= 0.0 and float(hi) >= 1.0:
self.grid_remove()
else:
self.grid()
tk.Scrollbar.set(self, lo, hi)
def test():
with open(__file__, "rU") as f:
root.mainFrame.text.insert("1.0", f.read())
if __name__ == "__main__":
root = MainWindow()
#test()
root.mainloop()
Update 1: I have written a new method response_handle
for the response handle to be used in 2 places in order to reduce repetition as suggested in this answer. Also for a similar reason I've written file_operation
and data_operation
methods to be used in save_file
and open_file
methods as they were very similar.
-
1\$\begingroup\$ This is CodeReview you post alternative code, but don't post a review of the code. codereview.meta.stackexchange.com/questions/8403/… \$\endgroup\$Ludisposed– Ludisposed2017年12月04日 17:17:22 +00:00Commented Dec 4, 2017 at 17:17
-
\$\begingroup\$ @Ludisposed I don't understand how this is unfitting in accordance to your link and your other link? \$\endgroup\$Nae– Nae2017年12月04日 17:32:30 +00:00Commented Dec 4, 2017 at 17:32
-
1\$\begingroup\$ You can answer your own code, but don't just dump it here. Explain your reasoning behind the changes should be enough. You can link to points in @alexce answer \$\endgroup\$Ludisposed– Ludisposed2017年12月04日 17:42:44 +00:00Commented Dec 4, 2017 at 17:42