Skip to content
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

Много всякого #6

Merged
merged 31 commits into from
Apr 28, 2023
Merged

Conversation

AndrewKraevskii
Copy link
Contributor

  • Удалена библиотека nom вместо неё ручной парсинг.
  • Замена макросов на функции в тех случаях когда это не несёт никакой пользы
  • Использование более современного синтаксиса для распаковки Optional
  • Убрано ручное индексирование в циклах for
  • Смена библиотеки для открытия файлов на более современную
  • Теперь файлы можно сохранять под любыми названиями а не только generic.mm
  • Исправлен краш программы при переполнении переменной (теперь там явно указано что переполнение желанное поведение)
  • [target.x86_64-pc-windows-msvc] передвинут в config.toml
  • Код форматирован с использованием cargo fmt
  • Большинство предупреждений исправлено или указано явно их игнорировать
  • cargo clippy предупреждения тоже исправлены

@AndrewKraevskii
Copy link
Contributor Author

Также теперь при запуске в debug режиме показывается консоль. Иначе rust не выводит ошибки

Copy link
Owner

@JustAGod1 JustAGod1 left a comment

Choose a reason for hiding this comment

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

Вроде все просмотрел. Но вот штуку с prev_prev_zero надо б все-таки вернуть.

src/ui/cells.rs Show resolved Hide resolved
src/ui/cells.rs Show resolved Hide resolved
@JustAGod1
Copy link
Owner

Спасибо за пулл реквест. Так видимо и буду учить расть.
Новые паттерн матчинги выглядят сексуально.
Зачем нужно было заменять парсинг на ручной мне не очень понятно. Я б заменил на chumsky или типа того, но соглашусь то как оно было до этого не может нравится адекватному человеку. Хоть и ручной парсинг не кажется адекватным решением, но видимо пусть будет так.
Выбор файла, а не папки, конечно, супер полезно.

@JustAGod1
Copy link
Owner

а ладно минуточка осознания, что prev_prev_zero никогда не обновлялся

@JustAGod1 JustAGod1 merged commit c2760ed into JustAGod1:master Apr 28, 2023
@AndrewKraevskii
Copy link
Contributor Author

AndrewKraevskii commented Apr 28, 2023

а ладно минуточка осознания, что prev_prev_zero никогда не обновлялся

Я на самом деле обнаружил что prevprev не нужен потому что компилятор раста сказал что она никогда не используется) Всё ещё удивляюсь возможностям статического анализа этого языка
image

@AndrewKraevskii
Copy link
Contributor Author

Честно в логике работы самого evm не разбирался (слишком страшно), так что изменения довольно поверхностные.

@AndrewKraevskii
Copy link
Contributor Author

Зачем нужно было заменять парсинг на ручной мне не очень понятно. Я б заменил на chumsky или типа того, но соглашусь то как оно было до этого не может нравится адекватному человеку. Хоть и ручной парсинг не кажется адекватным решением, но видимо пусть будет так.

Я когда смотрел код подумал что раз использование так лимитировано то почему бы и ручками не написать. Не планировал делать pull request так что просто попрактиковался в test driven development. Было давно так что я сам хз для чего сделал

AndrewKraevskii added a commit to AndrewKraevskii/bevm that referenced this pull request Apr 28, 2023
Merge pull request JustAGod1#6 from AndrewKraevskii/master
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.

None yet

2 participants