Repository navigation
add --stdin-filename option for ast generation on stdin - #27
Conversation
|
|
||
| for (args[1..]) |arg| { | ||
| const stat = try std.Io.Dir.cwd().statFile(io, arg, .{.follow_symlinks = true}); | ||
| for (args[fileArgumentsStart..]) |arg| { |
There was a problem hiding this comment.
Do we even need to support both stdin linting and by-name linting in the same command?
There was a problem hiding this comment.
Its needed so that if you provide a file via stdin, the linter still can know if its a zon / zig file. Else all rules which are based on the ast don't work via stdin.
There was a problem hiding this comment.
Or wait, I think I missunderstood your comment
There was a problem hiding this comment.
Okay, so there a 2 ways to provide a file: via the filename or via stdin. But via stdin we often want to still provide a name, so The linter can know if it should generate an ast or not.
Currently I allow providing --stdin-filename= and normal file names.. this is not needed / makes the logic more complicated. Either providing file or stdin. So I will change that. But we still need the --stdin-filename= option.
There was a problem hiding this comment.
Okay... I made it a bit cleaner now. I actually didn't allow both. I just had to allow continuing the function correctly because of the if at the end. (I just moved it to an defer now, so I can just return without forgetting it)
| for (args[fileArgumentsStart..]) |arg| { | ||
| const stat = std.Io.Dir.cwd().statFile(io, arg, .{.follow_symlinks = true}) catch |err| { | ||
| if (err == error.FileNotFound) { | ||
| std.log.err("FileNotFound: {s}", .{arg}); |
There was a problem hiding this comment.
I think it would make sense to print the file name for every error
There was a problem hiding this comment.
makes sense. Added a generic err log
| if (args.len <= 2) stdin: { | ||
| const fileNameOption = "--stdin-filename="; | ||
| const name = blk: { | ||
| if (args.len <= 1) break :blk "<stdin>"; |
There was a problem hiding this comment.
Should we even provide this option, when it's not really useful anyways?
There was a problem hiding this comment.
What do you mean with "not usefull anyways"?
There was a problem hiding this comment.
Well without providing the filename it doesn't show you any errors that require the ast, which doesn't really make it that useful.
There was a problem hiding this comment.
Ah you mean, if we should even allow the option of using stdin without the name? I think while a "correct" editor setup should give it, I would still just allow without it, because it doesn't that much complicate our logic
There was a problem hiding this comment.
Not allowing it would allow us to print an error prompting the user how to use it correctly.
There was a problem hiding this comment.
Okay you are right. This made me change quite a few things though...
./zig-out/bin/Cubyz-linter
error: Missing arguments.
Usage:
Cubyz-linter [FILE|DIRECTORY]... lint the files provided / files in the directories provided.
Cubyz-linter --stdin-filename=FILENAME lint the content provided via stdin, the filename is needed to know if its a zig / zon file.
Cubyz-linter -h --help print this help list
| if (args.len <= 2) stdin: { | ||
| const fileNameOption = "--stdin-filename="; | ||
| const name = blk: { | ||
| if (args.len <= 1) break :blk "<stdin>"; |
There was a problem hiding this comment.
Not allowing it would allow us to print an error prompting the user how to use it correctly.
2 participants
Fixes: #21
This could probably be better implemented with a generalized argument parser. But I don't think we will have that many. (probably not more than this). So I would like to have this be a done fix. and then maybe in the future make it better.
Also makes the error message on
FileNotFoundbetter, to show what argument it got. (I can remove that if wanted)