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

Add --include-path flag (#155) #213

Merged
merged 2 commits into from Dec 29, 2023
Merged

Add --include-path flag (#155) #213

merged 2 commits into from Dec 29, 2023

Conversation

ksg97031
Copy link
Member

Thank you for the feedback provided at #192 (comment)

I have incorporated your suggestions and made some adjustments:

  1. Renamed the Path struct to PathInfo.
    The Path struct had already been declared.

  2. Changed the --include-file-path flag to --include-path.
    I considered using --with-path, but it resembled the --with-headers flag and might cause confusion.

  3. Replaced the use of the paths variable with code_paths in the code.
    Using paths led to code like paths[0].path, which seemed a bit awkward. Therefore, I opted for code_paths as the variable name instead.

In the process of refactoring, several analyzer files now include the Details object for storing path and line number information. However, in a few cases where the line number couldn't be detected, only path information is stored.

I welcome any additional feedback!

- Add new properties and structs to the `Endpoint` model
- Add new flag options to the `noir` command line tool for various functionalities
- Add unit tests for the `Endpoint` model initialization and details detection
- Update the logic in the output builder to include file paths in the printed output if specified

Signed-off-by: ksg97031 <ksg97031@gmail.com>
- Refactored multiple analyzer files to include the `Details` object for storing path and line number information

Signed-off-by: ksg97031 <ksg97031@gmail.com>
Copy link
Member

@hahwul hahwul left a comment

Choose a reason for hiding this comment

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

The code is neat and straightforward, making it a pleasure to read. No difficulties whatsoever. Approving this excellent work :D

@hahwul hahwul added this to the v0.12.0 milestone Dec 29, 2023
@hahwul hahwul merged commit a891271 into dev Dec 29, 2023
3 checks passed
hahwul added a commit that referenced this pull request Dec 30, 2023
hahwul added a commit that referenced this pull request Dec 30, 2023
@hahwul hahwul mentioned this pull request Dec 30, 2023
@hahwul hahwul deleted the include-path-option branch January 4, 2024 14:55
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