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

Noir for VSCode Extension #192

Open
ksg97031 opened this issue Dec 10, 2023 · 5 comments
Open

Noir for VSCode Extension #192

ksg97031 opened this issue Dec 10, 2023 · 5 comments

Comments

@ksg97031
Copy link
Member

No description provided.

@ksg97031
Copy link
Member Author

vscode command for file navigation (https://code.visualstudio.com/api/references/commands)

vscode.commands.executeCommand(
  "editor.action.goToLocations",
  vscode.Uri.file(filepath),
  new vscode.Position(linenumber, 0),
  [],
  "goto"
);

ksg97031 added a commit that referenced this issue Dec 23, 2023
- Modified `Endpoint` struct by adding a new `details` property and an initialization method
- Modified `Details` struct by adding `path` and `line` properties and an initialization method
- Improved default options in `options.cr` by adding a new `include_file_path` option

Signed-off-by: ksg97031 <[email protected]>
@ksg97031
Copy link
Member Author

Hello @hahwul !

I've added a new option to include file paths in the result output. I did this by adding a details field (a struct type) to the Endpoint to store the path and line number information. You can check out the changes in this commit:

https://github.com/noir-cr/noir/tree/include-path-option 745652b

I'd appreciate your feedback on this approach. Do you think it's suitable, or are there better ways to achieve this functionality?

Here are examples of how the output looks with the new option now:

  • with --include-file-path option
    image

  • with -json option
    image

Please give me feedback whenever you have time. No rush.

@hahwul
Copy link
Member

hahwul commented Dec 23, 2023

Hiya @ksg97031
Wow! It''s awesome.

Regarding feedback, how about allowing the provision of multiple file paths instead of just one? Sometimes, an application's endpoint can be declared from multiple files, so it would be great to consider accommodating such cases by providing an array format for file paths. I think it could contribute to greater flexibility and adaptability, in my opinion. What are your thoughts on this idea?

And I think the name 'Details' could potentially hold more than just 'File' and 'Line' information. So, I've made some improvements below, and I'm curious to hear your thoughts on this.

e.g

struct Details
  include JSON::Serializable
  include YAML::Serializable
  @paths : Array(Path) = [] of Path
  # + New details types to be added in the future..

  def add_path(path : Path)
    @paths << path
  end
end

struct Path
  include JSON::Serializable
  include YAML::Serializable
  property path, line

  def initialize(@path : String, @line : Int32 | Nil)
  end
end
[
  {
    "headers": [],
    "method": "POST",
    "params": [],
    "protocol": "http",
    "url": "https://testapp.internal.domains/comments",
    "details": {
       "paths": [
          {"path":"/source/123.java", "line": 35},
          {"path":"/source/123.java", "line": 124},
          {"path":"/route/route.java", "line": 12},
        ] 
     }
  }
]

@hahwul
Copy link
Member

hahwul commented Dec 23, 2023

@ksg97031
I really like the flag name you chose, --include-file-path! I had also been considering a list similar to the one below for flag names in my mind, so it seems like our thoughts align.

  • --include-path
  • --with-path

I also think it might be a good idea to consolidate plain and JSON outputs into a single flag. For instance, when using the --include-file-path flag, if the format is default, we could output it under the 'Endpoint' as shown in the image. In the case of JSON(-f json), we could include it in the details for json output.

@hahwul
Copy link
Member

hahwul commented Dec 24, 2023

@ksg97031
Anyway, looking forward to your great ideas and opinions! Have a wonderful Christmas bro 🎄

ksg97031 added a commit that referenced this issue Dec 28, 2023
- Modified various analyzer classes to include a `Details` object in each `Endpoint` for better endpoint details tracking.
- Updated the output builder to properly display multiple file paths and line numbers in the output.

Signed-off-by: ksg97031 <[email protected]>
ksg97031 added a commit that referenced this issue Dec 29, 2023
- Refactored multiple analyzer files to include the `Details` object for storing path and line number information
- Updated the `Endpoint` struct to include a new method `set_details` and added new structs `Details` and `PathInfo`

Signed-off-by: ksg97031 <[email protected]>
ksg97031 added a commit that referenced this issue Dec 29, 2023
- Refactored multiple analyzer files to include the `Details` object for storing path and line number information

Signed-off-by: ksg97031 <[email protected]>
ksg97031 added a commit that referenced this issue Dec 29, 2023
- Refactored multiple analyzer files to include the `Details` object for storing path and line number information

Signed-off-by: ksg97031 <[email protected]>
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

No branches or pull requests

2 participants