Skip to content

Conversation

@pvktk
Copy link
Owner

@pvktk pvktk commented Mar 1, 2018

No description provided.

…sign into waiting_review

µния, зачем нужно
Copy link

@yurii-litvinov yurii-litvinov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В общем-то, серьёзных замечаний нет, и вроде всё работает. Зачтена.

#Класс, вызывающий внешние комманды
class system_commands:
def run(self, pipe_arg, command_name, arglist):
#print('command for execute: ', [command_name] + arglist)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вообще, закомментированный код лучше не оставлять, в процессе рефакторинга код вокруг может поменяться, и когда его раскомментируют, он уже не будет работать.

#У этих классов есть метод run_on, в который передаётся pipe в виде строки и
#аргументы в виде списка. Возвращает строку-результат

#Команда 'cat'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Мне казалось, что комментарии к функциям и классам правильнее писать через """

#Команда 'pwd'
class pwd_command:
def run_on(self, pipe_arg, arglist):
return os.getcwd()+'\n'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Бинарные операторы отделяются пробелом (ну, собственно, в остальном коде так и есть)

'wc' : wc_command(),
'pwd' : pwd_command()}
self.sys_comm = system_commands()
#метод для исполния распарсенной строки. Возвращает результат в виде строки.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"исполния", опечатка

try:
print(lexec.exec_line(inpline), end='')
except:
exit()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Мне кажется, даже для Питона всё в одном файле --- это слишком брутально. И точно вызвало бы проблемы с масштабируемостью при развитии решения. Имеет смысл распилить по нескольким файлам.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants