Python File Explorer / RSync Terminal User Interface

Question

I've been working on my first python program for a few weeks now and I feel like the program is at a decent stage to begin sharing. The premise was to create a file explorer that eases the use of rsync. The reason I've chosen rsync over ssh/sftp is due to my current location and extremely limited internet connectivity/high-latency. This was inspired by midnight commander but is not written in C and I have not used any code from MC.

However, as I continue to develop the application, I feel as if I can be much more efficient with the code structure, and probably even the use of ssh when pulling certain file attributes. I've worked on refactoring a few times but am definitely aware the program needs a lot of love.

Goals: refactoring, use rsync over an established ssh channel, and more bandwidth efficient.

Question: what is the best option to help thin out this code - should I be relying on decorators? Or, should my classes and methods be much smaller?

For example - to make the primary file explorer I utilize a loop to print the list of directory's (menu method), and then navigate through it (navigate method) and constantly keep reprinting the list depending on the key press (display method). However, I continue to use a similar code structure throughout the application to make a menu and buttons. There are small changes in each of those iterations of the code to adapt them for their uses. Here's the primary structure I'm referring to.

class file_explorer:
    """Work horse of the application

    Does the bulk of the work by taking a path and listing directorys/files within that 
    path. Also prints the list of directorys/files and is responsible for navigating
    through those files. The ssh_explorer method is meant to closely mirror the explorer
    method but is modified for paramiko where appropriate.
    """
    path = prev_path_0 = prev_path_1 = '/'
    paths = [None,None]
    prev_paths = [prev_path_0, prev_path_1]
    path_errors = reset_path()
    def __init__(self, stdscr, window, path, is_ssh):
        self.marked_item = None
        self.window = window
        self.screen = stdscr
        self.screen_height, self.screen_width = self.screen.getmaxyx()
        height, width = self.window.getmaxyx()
        start_y, start_x = self.window.getbegyx()
        self.p = 0
        self.height = height
        self.width = width - 2
        self.start_y = start_y + 1
        self.start_x = start_x + 1
        self.position = 0
        self.scroller = 0
        self.explorer(path)
        self.draw_pad()
        self.event = ''
        self.selected_path = ''
        self.ssh_path = None
        self.ssh_path_hist = ['/']

    def explorer(self, path):
        if wins.active_panel == 1 and ssh_obj.enabled == True:
            #self.ssh_path = path
            file_explorer_1.ssh_explorer(path)
            return 0
        self.path = path
        try:
            self.abs_path = os.path.abspath(self.path)
            self.par_dir = os.path.dirname(self.abs_path).replace('//','/')
        except Exception as e:
            pass
        try:      
            self.files_folders = os.listdir(self.abs_path)
        except:
            self.abs_path = '/'
            self.files_folders = os.listdir(self.abs_path)
            self.path_errors.error()
            pass
        data_list = []
        for x in self.files_folders:
            i = os.path.isdir(self.abs_path + '/' + x)
            try:
                s = os.path.getsize(self.abs_path + '/' + x)
            except:
                s = 0
            s = human_readable_size(s, suffix="B")
            if i == True:
                i = '/'
            else:
                i = ''
            data_list.append([i+x, s])
        #sort list based on file names
        data_list = sorted(data_list, key=lambda x:x[:1])
        #insert an index
        i = 1
        for x in data_list:
            x.insert(0,i)
            i = i + 1
        #turn data list into a dictionary
        x = 0
        self.data = dict()
        self.data = {x[0]: x[1:] for x in data_list}
        if self.abs_path == '/':
            self.data[0] = ['/','']
        else:
            self.data[0] = ['..','']

    def ssh_path_hist_func(self, ssh_path):
        #since paramiko doesnt produce an absolute path, this creates a path
        #history by appending and pop'ing an array as you move through the file
        #structure
        if ssh_path == '/':
            self.par_dir = '/'
            self.ssh_path = ssh_path
            self.ssh_path_par_hist = ['/']
            self.ssh_path_hist = ['/']
            self.p = 0
            pass
        elif ssh_path.startswith('/'):
            self.ssh_path = ssh_path.lstrip('.')
            self.ssh_path_hist.append(ssh_path)
            self.p +=1
            self.ssh_path_par_hist = list(self.ssh_path_hist)
            self.ssh_path_par_hist.pop()
            self.par_dir = ''.join(map(str, self.ssh_path_par_hist))

        elif ssh_path.startswith('.') and len(self.ssh_path_hist) > 1:

            self.ssh_path_hist.pop(self.p)
            self.p -=1
        else:
            self.par_dir = '/'
            if len(self.ssh_path_par_hist) != 1:
                self.ssh_path_par_hist.pop()
                self.par_dir = ''.join(map(str, self.ssh_path_par_hist))

        self.next_ssh_path = ''.join(map(str, self.ssh_path_hist))

    def ssh_explorer_attr(self):
        if glbl_opts.low_bandwidth == True:
            s = 0
        else:
            self.ssh_files_folders_attr = ssh_obj.sftp.listdir_attr(
                path=self.next_ssh_path)
            size_list = []
            for entry in self.ssh_files_folders_attr:
                s = entry.st_size

        return size_list

    def ssh_get_abs_path(self, path):
        try:
            self.ssh_abs_path = ssh_obj.sftp.normalize(path) 
        except:
            pass

    def ssh_explorer(self, ssh_path):
        self.ssh_path_hist_func(ssh_path)
        self.ssh_files_folders_dir = ssh_obj.sftp.listdir(
                path=self.next_ssh_path
                )
        self.ssh_files_folders_attr = ssh_obj.sftp.listdir_attr(
                path=self.next_ssh_path
                )
        self.ssh_get_abs_path(self.next_ssh_path)
        item = 0
        data_list = []
        for x, entry in zip(self.ssh_files_folders_dir, self.ssh_files_folders_attr):
            i = S_ISDIR(entry.st_mode)
            s = entry.st_size
            if i == True:
                i = '/'
            else:
                i = ''
            data_list.append([i+x, s])
            item += 1
        #sort list based on file names
        data_list = sorted(data_list, key=lambda x:x[:1])
        #insert an index and use the size function to make size human readable
        i = 0
        for x in data_list:
            i += 1
            x.insert(0,i)
            s = human_readable_size(x[2], suffix="B")
            x[2]=s
        #turn data list into a dictionary
        x = 0
        self.data = dict()
        self.data = {x[0]: x[1:] for x in data_list}
        if self.abs_path == '/':
            self.data[0] = ['/','']
        else:
            self.data[0] = ['..','']

    def draw_pad(self):
        self.pad = curses.newpad(self.height + 800, self.width) #size of pad

        self.pad.scrollok(True)
        self.pad.idlok(True)
        self.pad.keypad(1)
        self.pad.bkgd(curses.color_pair(4))

    def select_item(self,mylist, x, v):
        item_to_edit = mylist[x]
        item_index = mylist.index(item_to_edit)

        for index, item in enumerate(mylist):
            itemlist = list(item)
            if index == item_index:
                itemlist[1] = v
            item = tuple(itemlist)
            mylist[index] = item
        self.tup = mylist
        return mylist

    def deselect_item(self,mylist, x, v):
        item_to_edit = mylist[x]
        item_index = mylist.index(item_to_edit)
        for index, item in enumerate(mylist):
            itemlist = list(item)
            if index == item_index:
                itemlist[1] = v
            item = tuple(itemlist)
            mylist[index] = item
        self.tup = mylist
        return mylist

    def get_selected_items(self, tup):
        new_list = []
        i = -1
        for ind, sel, item in tup:
            
            if sel == '[x]':
                i += 1
                tups = tuple((i,item))
                new_list.append(tups)
        self.marked_item = self.path + '/' + new_list[0][1]

    def del_selected_items(self, sel_file):
        PopUpDelete(sel_file)

    def copy_selected_items(self):
        file_name = self.data[self.position][0]
        left_panel_path = file_explorer_0.abs_path
        right_panel_path = file_explorer_1.abs_path
        if wins.active_panel == 0:
            self.from_file = self.path + '/' + file_name
            self.to_path = file_explorer_1.path
        elif wins.active_panel == 1:
            self.from_file = self.path + '/' + file_name
            self.to_path = file_explorer_0.path
        if self.position != 0:
            PopUpCopyFile(stdscr, self.from_file, self.to_path, file_name)

    def start_rsync(self):
        file_name = self.data[self.position][0]
        left_panel_path = file_explorer_0.abs_path
        right_panel_path = file_explorer_1.ssh_abs_path
        if wins.active_panel == 0:
            self.from_file = left_panel_path + '/' + file_name
            self.to_path = right_panel_path
        elif wins.active_panel == 1:
            self.from_file =  right_panel_path + '/' + file_name
            self.to_path = left_panel_path
        if self.position != 0:
            rsync_obj = RSync(0).start(self.from_file, self.to_path, file_name)

    def menu(self):
        self.pad.erase()
        self.height, self.width = self.window.getmaxyx()
        self.screen_height, self.screen_width = self.screen.getmaxyx()

        self.max_height = self.height -2
        self.bottom = self.max_height #+ len(self.tup) #self.max_height
        self.scroll_line = self.max_height - 3
        self.pad.setscrreg(0,self.max_height) #self.bottom -2)
        self.width = self.width - 2
        self.pad_refresh = lambda: self.pad.noutrefresh(self.scroller, 
        0, self.start_y, self.start_x, self.bottom, self.screen_width -2)
        #par = '[ ]' # can be added to the msg below to create a selector, likely to be removed
        for index, items in self.data.items():
            padding = self.width - len(items[0]) - 5
            if index == self.position:
                mode = curses.A_REVERSE
            else:
                mode = curses.A_NORMAL
            try:
                msg = f'{index:>3}{" "}{items[0]}{items[1]:>{padding}}'
            except:
                msg = f'{index:>3}{" "}{items[0]}'
            self.pad.addstr(index, 0, str(msg), mode)
            if mode == curses.A_REVERSE:
                self.cursor = self.pad.getyx()[0]
        self.pad_refresh()

    def navigate(self, n):
        self.position += n
        if self.position < 0:
            self.position = 0
        elif self.position >= len(self.data):
            self.position = len(self.data) - 1
    
    def set_paths(self):
        x = wins.active_panel
        if x == 0:
            oth_panel = 1
        else:
            oth_panel = 0
        if self.new_path == None:
            self.paths[x] = self.prev_paths[x].replace('//','/')
        else:
            self.prev_paths[x] = self.paths[x]
            self.paths[oth_panel] = self.prev_paths[oth_panel]

    def display(self):
        while True:
            self.pad.keypad(1)
            self.menu()
            curses.doupdate()
            KEY_PRESS = None
            try:
                #self.pad.keypad(1)
                KEY_PRESS = self.pad.getch()
            except KeyboardInterrupt:
                KEY_PRESS = ord('q')
                self.event = KEY_PRESS
                break
            if KEY_PRESS == ord("\n"):
                if '/' in self.data[self.position][0]:
                    itsadir = True
                else:
                    itsadir = False
                if self.position != 0 and itsadir == True:
                    if ssh_obj.enabled == True and wins.active_panel == 1:
                        self.new_path =  self.data[self.position][0] + '/'
                    else:
                        self.new_path = self.path + self.data[self.position][0] + '/'
                    self.cwd = self.new_path
                    #self.selected_path = self.path + self.data[self.position][0] + '/'
                    self.pad_refresh()
                    self.position = self.scroller = 0
                    #self.event = KEY_PRESS
                    self.paths[wins.active_panel] = self.new_path.replace('//','/')
                    self.set_paths()
                    self.explorer(self.new_path)
                    wins.upd_panel()
                elif self.position != 0 and itsadir == False:
                    sel_file = self.path + '/' + self.data[self.position][0]
                    PopUpFileOpener(sel_file, KEY_PRESS,self.screen, None)
                else:
                    
                    self.cwd = self.new_path = self.par_dir
                    if ssh_obj.enabled == True and wins.active_panel == 1:
                        self.par_dir = '..'
                    self.set_paths()
                    self.explorer(self.par_dir)
                    wins.upd_panel()
            elif KEY_PRESS == curses.KEY_UP:
                self.navigate(-1)
                if self.cursor > self.scroll_line:
                    self.scroller  -= 1
                    if self.scroller == -1:
                        self.scroller = 0
                if self.position == 0:
                    self.scroller = 0
            elif KEY_PRESS == curses.KEY_DOWN:
                self.navigate(1)
                if len(self.data) == 1:
                    self.position = 0
                elif self.cursor >= self.scroll_line:
                    if self.position != len(self.data):
                        self.scroller = self.cursor - self.scroll_line
            elif KEY_PRESS == ord('n'):
                popUpNewDir()
            #elif KEY_PRESS == ord('m'):
            #    if self.position != 0:
            #        self.select_item(self.tup, self.position, '[x]')
            #elif KEY_PRESS == ord('u'):
            #    if self.position != 0:
            #        self.deselect_item(self.tup, self.position, '[ ]')
           #elif KEY_PRESS == ord ('g'):
            #    self.get_selected_items(self.tup)
            elif KEY_PRESS == ord('b'):
                self.position = len(self.data)-1
                self.scroller = len(self.data) - self.scroll_line -1                    
            elif KEY_PRESS == ord('t'):
                self.position = 0
                self.scroller = 0
            elif KEY_PRESS == ord('9'):
                self.del_selected_items(self.path + '/' + self.data[self.position][0])
            elif KEY_PRESS == ord('5'):
                if ssh_obj.enabled == False:
                    self.copy_selected_items()
                    reset_window(file_explorer_0)
                    reset_window(file_explorer_1) 
                elif ssh_obj.enabled == True:
                    self.start_rsync()
            elif KEY_PRESS == ord('x') and ssh_obj.enabled == True:
                ssh_obj.ssh.close()
                ssh_obj.enabled = False
                wins.upd_panel()
                file_explorer_1.explorer(file_explorer_1.path)
                file_explorer_1.menu()
                reset_window(file_explorer_1)
            elif KEY_PRESS in(ord('f'), ord('o'), ord('q'), ord('\t')):
                #status.refresh()
                self.event = KEY_PRESS
                break

Another example of this similiar structure but for the menubar drop downs...

class MenuBar(PopUpBase):
    """Creates the menubar

    Used to create either the File or Options menu, draws a window depending
    on if f or o is called.
    """
    position = 0
    sub_menu = ['ssh','rsync','exit']
    sub_menu2 = ['settings']
    menubarwindow = ''
    def __init__(self):
        stdscr.addstr(0,1, 'File Options', curses.A_NORMAL)
        stdscr.noutrefresh()
    
    def menubar_act(self, menu_event):
        if menu_event == ord('f'):
            super().__init__(10, 15, 1, 1)
            #now using self.win from base
            stdscr.addstr(0,1, 'File', curses.A_STANDOUT)
            stdscr.addstr(0,4+2, 'Options', curses.A_NORMAL)
            stdscr.noutrefresh()
            self.menu_item = self.sub_menu

        if menu_event == ord('o'):
            super().__init__(10, 15, 1, 6)
            stdscr.addstr(0,1, 'File', curses.A_NORMAL)
            stdscr.addstr(0,4+2, 'Options', curses.A_STANDOUT)    
            stdscr.noutrefresh()
            self.menu_item = self.sub_menu2
        
    def menu(self):
        #print list as basis for menu, the item selector is the A_REVERSE (highlighted) item
        for index, item in enumerate(self.menu_item):
            if index == self.position:
                mode = curses.A_REVERSE
            else:
                mode = curses.A_NORMAL
            self.win.addstr(1+ index, 1, item, mode)
        self.win.noutrefresh()
        curses.doupdate()

    def noutrefresh(self):
        stdscr.addstr(0,1, 'File Options', curses.A_NORMAL)
        stdscr.noutrefresh()

    def navigate(self, n):
        self.position += n
        if self.position < 0:
            self.position = 0
        elif self.position >= len(self.menu_item):
            self.position = len(self.menu_item) - 1

    def display(self):
        self.win.keypad(1)
        while True:
            self.menu()
            ### menuBar getch
            ch = self.win.getch()
            if ch == curses.KEY_UP:
                self.navigate(-1)
                if self.position < 0:
                    self.position = 0
            elif ch == curses.KEY_DOWN:
                if self.position >= 0 and self.position <= len(self.menu_item) - 1:
                    self.navigate(1)
            elif ch == ord('\n') and self.menu_item[self.position] == 'ssh':
                if ssh_obj.enabled is False:
                    self.win.erase()
                    del self.win
                    reset_window(file_explorer_0)
                    ssh_obj.start()
                    return 0
            elif ch == ord('\n') and self.menu_item[self.position] == 'rsync':
                self.win.erase()
                del self.win
                reset_window(file_explorer_0)
                reset_window(file_explorer_1)
                RSync('m')
                return 0
            elif ch == ord('\n') and self.menu_item[self.position] == 'settings':
                self.win.erase()
                del self.win
                reset_window(file_explorer_0)
                glbl_opts.display()
                return 0
            elif ch == ord('\n') and self.menu_item[self.position] == 'exit':
                ch = ord('q')
                return ch
            elif ch in (ord('f'), ord('o'), ord('\t'), ord('q')):
                stdscr.addstr(0,1, 'File', curses.A_NORMAL)
                stdscr.addstr(0,4+2, 'Options', curses.A_NORMAL)
                stdscr.noutrefresh()
                curses.doupdate()
                reset_window(file_explorer_0)
                return ch

I'd like to make this more efficient, possibly relying on decorators but then I end up having to try and account for unique button presses depending on the "menu". I've used base classes for various pop up windows so I don't have to rewrite the same window multiple times and just call super() but again, I think I might be missing out on some deeper concepts in Python that can help me to improve the code/portability/reusability.

I've included a link to the full code below and am happy to share if you want to use it. The code is about 1500 lines so I did not think it would be smart to copy it all in. https://github.com/1amdash/syncrpy-tui/blob/main/syncrpy-tui-55.py

Answer 1

The main problem with this code is that it's very hard to comprehend. This review will focus on this issue exclusively, not on the logic of the code itself. Once your code is more clean and 'pythonic' it will be much easier to see the problems with ssh channels, bandwidth, etc..

import os
import sys
import time
import curses
import io
import paramiko
import shutil
import threading
import subprocess
import shlex
import tempfile
from curses import panel, textpad
from stat import S_ISDIR, S_ISREG
from getpass import getpass

Give each class its own file and import only things you need in the current file. Don't use import and from ... import statements for the same module, it leads to confusion:

curses.doupdate()  # ok, this is a function from curses module
textpad  # and where did this come from? oh, it is also from curses

In fact none of the separately imported names (panel and textpad) are ever used on their own rendering the import line useless.

Let's jump to the end of the file:

...
menu = MenuBar()
wins = WinManager(stdscr)
start_path = os.path.curdir
window_0 = wins.left_panel()
window_1 = wins.right_panel()
window_0.noutrefresh()
window_1.noutrefresh()
ssh_obj = SSH()
file_explorer_0 = file_explorer(stdscr, window_0, start_path, False)
file_explorer_1 = file_explorer(stdscr, window_1, start_path, False)
file_explorer_0.menu()
file_explorer_1.menu()
paths = []
event = 0
i = 0
...

We have some code that is supposed to run when you run the file. Use if __name__ == '__main__': construction for this purpose.

Let's read some lines from above.
menu = MenuBar()
So is it menu bar or just menu?
wins = WinManager(stdscr)
I can't tell what wins is supposed to mean by looking at the name.
window_0 = wins.left_panel()
window_1 = wins.right_panel()
What's the difference between window_0 and window_1? I can see it only at the assignment, nowhere else.
window_0.noutrefresh()
window_1.noutrefresh()
Can't make out what the method name says, use snake_case for names.
file_explorer_0 = file_explorer(stdscr, window_0, start_path, False)
Why is it 0?
What does this method do? Oh, wait it's a class constructor! Use CamelCase for class names.
What is False argument supposed to mean? Use named arguments for booleans and in other ambiguous cases, e.g. is_ssh=False in this case.
file_explorer_0.menu()
What does this method do?
Call methods according to what their actual purpose is.
paths = []
event = 0
i = 0
Once again, no idea what any of these variables are.
The main takeaway from this section: use proper naming. It makes code significantly more readable.

del window_0
del window_1
del status
Don't use del without a good reason, python has a garbage collector.

if ssh_obj.enabled == True: is equal to if ssh_obj.enabled:

try: 
    self.des_num = os.path.getsize(self.full_qual_dest_path)
    self.des_size = human_readable_size(self.des_num)
except:
    pass

Don't ever ignore exceptions like that. You're letting every possible error just pass unnoticed. I can't tell what the right way here is since I don't even know what exception you're expecting.

try:
    self.des_num = int(float(os.path.getsize(self.full_qual_dest_path)))
    self.des_size = human_readable_size(self.des_num)
except:
    time.sleep(.05)
    self.des_num = int(float(os.path.getsize(self.full_qual_dest_path)))
    self.des_size = human_readable_size(self.des_num)

If it failed the first time why do you think it won't fail after you wait for .05? What is the cause of the problem?

elif KEY_PRESS == 10 and self.position == 1:
    self.status = 0
    self.action = 1

What do any of those numbers mean? Use constants or enums instead of plain numbers to convey such information.

There is much more to cover but I think fixing problems I pointed out will be a good starting point. To sum it up:

• Meaningful names: names should convey the meaning, it should be clear what information fields contain, what functions do, what classes are for by looking at their names
• Project structure: split your code into files to make it less crowded and ideally less interdependent
• Exceptions handling: don't silence exceptions, always show what exceptions you're expecting for in a try block
• Magical numbers: don't leave undocumented numbers in your code, it's impossible to know what a particular number means

Good luck!