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

modified the tables fit on a 120 character terminal #1080

Merged

Conversation

hitenkoku
Copy link
Collaborator

@hitenkoku hitenkoku commented Jun 3, 2023

What Changed

  • changed rule authors table with 3 column
  • limited output char size in alerts table to 50chars

I would appreciate it if you could review when you have time.

@hitenkoku hitenkoku added the enhancement New feature or request label Jun 3, 2023
@hitenkoku hitenkoku self-assigned this Jun 3, 2023
@hitenkoku hitenkoku linked an issue Jun 3, 2023 that may be closed by this pull request
@hitenkoku
Copy link
Collaborator Author

Evidence

> 1071.exe csv-timeline -d ../hayabusa-sample-evtx  -q
image image

@YamatoSecurity YamatoSecurity added this to the v2.6.0 milestone Jun 3, 2023
@codecov
Copy link

codecov bot commented Jun 3, 2023

Codecov Report

Patch coverage: 9.37% and project coverage change: -0.06 ⚠️

Comparison is base (57df419) 81.84% compared to head (013f8d6) 81.78%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1080      +/-   ##
==========================================
- Coverage   81.84%   81.78%   -0.06%     
==========================================
  Files          24       24              
  Lines       19989    20002      +13     
==========================================
- Hits        16359    16358       -1     
- Misses       3630     3644      +14     
Impacted Files Coverage Δ
src/afterfact.rs 66.67% <6.45%> (-0.39%) ⬇️
src/main.rs 64.38% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@YamatoSecurity
Copy link
Collaborator

@hitenkoku Thank you!
Is it possible to check the size of the terminal and perform the following to dynamically adjust the number of columns accordingly?

  • If terminal.width <= 129 characters then print 3 columns
  • If terminal.width == 130 ~ 159 characters then print 4 columns
  • If terminal.width == 160 ~ 190 characters then print 5 columns
  • If terminal.width > 191 characters then print 6 columns

@hitenkoku
Copy link
Collaborator Author

Thank you for your check.

Is it possible to check the size of the terminal and perform the following to dynamically adjust the number of columns accordingly?

  • If terminal.width <= 129 characters then print 3 columns
  • If terminal.width == 130 ~ 159 characters then print 4 columns
  • If terminal.width == 160 ~ 190 characters then print 5 columns
  • If terminal.width > 191 characters then print 6 columns

Ok, I'll try it.

@hitenkoku
Copy link
Collaborator Author

The display was broken when the table had 5 columns at 165 characters, so I modified the table to have 4 columns up to 169 characters in cb4e831 .

@YamatoSecurity
Copy link
Collaborator

@hitenkoku Thanks again. Mac terminalではテーブルが壊れる場合があるので、条件の幅の微調整をしています。少々お待ちください。
アラート名の最大文字数も動的に調整できますか?例えば、幅が120文字の場合は (120 / 2 ) - 10 = 50 ( - 10のところを試行錯誤で試す必要があるので、こちらで検証しておきます)

@hitenkoku
Copy link
Collaborator Author

@YamatoSecurity 9d99f9e で以下の対応が完了しました。ご確認をお願いいたします。

@YamatoSecurity
Copy link
Collaborator

@hitenkoku ありがとうございます!ターミナルの幅を狭くしても、テーブルが崩れないで良い感じです:
Screen Shot 2023-06-04 at 4 54 11 PM

幅が十分広くても、最大数が50文字?で最後が...と切れています。幅が十分の場合はタイトルを全部出力したいのですが、可能でしょうか?

Screen Shot 2023-06-04 at 5 00 37 PM

@hitenkoku
Copy link
Collaborator Author

はい、大丈夫です。少々お待ちください

@hitenkoku
Copy link
Collaborator Author

対応完了しました。十分に長い文字列数の設定を最大値に設定したためこれで省略されることはなくなったかと思います(200文字を最大値に修正)

Copy link
Collaborator

@YamatoSecurity YamatoSecurity left a comment

Choose a reason for hiding this comment

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

@hitenkoku ありがとうございました!とても良い感じです。LGTM.

src/main.rs Outdated Show resolved Hide resolved
src/afterfact.rs Outdated Show resolved Hide resolved
@hitenkoku
Copy link
Collaborator Author

迅速な確認ありがとうございます!それではマージさせていただきます

@hitenkoku hitenkoku merged commit 7ec306c into main Jun 4, 2023
@hitenkoku hitenkoku deleted the 1071-make-sure-the-tables-fit-on-a-120-character-terminal branch June 4, 2023 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make sure the tables fit on a 120 character terminal
2 participants