WIP: Verify paths to be writable #400
Conversation
To resolve the regression from fsfe#347 this code checks if files can be written and throws an error otherwise. Signed-off-by: Nico Rikken <nico@nicorikken.eu>
|
Please consider this a work in progress or suggestion. This version works like: It might be more user-friendly to switch to an explicit license file directly, but that would require more rework in the general Note: this does not yet include a unit test or translation. |
This unit test checks if an error is raised when a file cannot be written to. This test is currently not very discriminating as it would also succeed for other errors. Signed-off-by: Nico Rikken <nico@nicorikken.eu>
|
I added a unit test, but that one needs more work, as it is too generic. It would also succeed if this code change wouldn't be made as the regression would also result in an error. |
Incorporates good ideas from pr fsfe#400. I didn't see it existed when I created this pr. - _check -> _verify to match naming style of other verification functions. - split the parameters into paths and parser
There was a problem hiding this comment.
I've adopted the neater function name and signature of this pr in #418 and fixed the case of passing --style.
| @@ -553,6 +564,7 @@ def run(args, project: Project, out=sys.stdout) -> int: | |||
|
|
|||
| # Verify line handling and comment styles before proceeding | |||
| if args.style is None and not args.explicit_license: | |||
| _verify_writable(paths, args.parser) | |||
There was a problem hiding this comment.
I overlooked this pr when I created pr #418, which does almost exactly the same, except for when the check is done.
Isn't the check needed even when args.style is set to a certain comment-style?
There was a problem hiding this comment.
It is. I've added a --style in the #418 test.
| if not os.access(path, os.W_OK): | ||
| parser.error( | ||
| _( | ||
| "'{path}' is not writable, please use --explicit-license" |
There was a problem hiding this comment.
In #418, I opted for not breaking on first error, but collecting all non-writable paths and mentioning them all at once, so the user can react to the complete set of problems.
Also, #418 doesn't mention --explicit-license but reuses the existing "can't write to'{}'" string that is already translated to different languages. And I can imagine cases where chmod is a better solution than --explicit-license
Incorporates good ideas from pr fsfe#400. I didn't see it existed when I created this pr. - _check -> _verify to match naming style of other verification functions. - split the parameters into paths and parser
|
This pull request has become obsolete. |
To resolve the regression from #347 this code checks if files can
be written and throws an error otherwise.
Signed-off-by: Nico Rikken nico@nicorikken.eu