-
Notifications
You must be signed in to change notification settings - Fork 1
add cdls #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: bash
Are you sure you want to change the base?
add cdls #4
Conversation
ArtyomLobanov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Интересная характеристика
В целом все прикольно
- Сообщения об ошибках можно было бы и информативнее сделать
>cd olo
File not found error
>cd build.gradle
File not found error- Адаптируйте внешние команды к своему пониманию рабочей директории
>cd ../../..
>pwd
C:\practice
# В этой папке нет git репозитория
>git status
On branch cdls
Your branch is up-to-date with 'origin/cdls'.
Untracked files:
(use "git add <file>..." to include in what will be committed)
bash.iml
nothing added to commit but untracked files present (use "git add" to track)| * @return full path | ||
| */ | ||
| fun getPath(path: String): String { | ||
| return getFile(path).canonicalPath |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Работать с путями как со строками в 2k19 довольно странно - есть Path
| File(path).canonicalFile | ||
| } else { | ||
| File(curDir, path).canonicalFile | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А в Path эта логика уже реализована (Path.resolve)
| val path = arguments.getOrElse(0) { "./" } | ||
|
|
||
| if (!env.updateDir(path)) { | ||
| output.write("File not found error") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не надо смешивать вывод команд и сообщения об ошибках. Бросайте исключения
| return | ||
| } | ||
|
|
||
| val path = arguments.getOrElse(0) { "./" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В bash если cd не передать аргументов, то рабочая директория меняется на домашнюю
|
@ArtyomLobanov подправил, работает |
|
Добавьте зависимость от JUint в build.gradle |
|
Хм, у меня заработало, я наверное подумал что он есть в исходнике |
|
добавил |
yurii-litvinov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Мне всё нравится, зачтена
В целом все прикольно, пришлось добавить смену директории в окружении, но это действительно не требовалось в исходной задаче, можно было бы до бесконечности бы писать "на всякий случай".
Но раз это добавилось, то легко увидеть, что на самом деле почти все команды(кроме echo, и то это только в рамках задачи опять же), должны иметь возможность обращаться к своему окружению, поэтому я не делал бы дополнительную абстракцию с environment command.
Понравилось, что команду делать легко - добавить в фабрику и дописать класс, также понравилось что есть отдельный класс для окружения и предусмотрено, что команды должны о нем знать.
Моментов, которые не понравились, нет в общем то, у меня добавление команд затруднений и лишней работы не вызвало, разве что в cat есть какие-то проблемы кажется.