[FEATURE] Allow overwrite existing files, add flags for permission handling #2009

Irallia · 2020-08-06T10:45:59Z

Please do not merge before the naming discussion of the enum on gitter has ended.

Existing Ideas:

enum output_file_open_mode
{
   overwritable / rewritable,
   exclusive
};

There is something in system.io from Microsoft that sounds good: https://docs.microsoft.com/de-de/dotnet/api/system.io.filemode?view=netcore-3.1 :

enum FileMode
{
    open_or_create,
    create_new
};

enum accept_existing_file
{
    true / yes,
    false / no
};

Rust:

enum OpenOptions
{
    create,
    create_new
};

2020-08-31

Discussion result:

enum output_file_open_options
{
    open_or_create,
    create_new
};

You call this validator like:

seqan3::output_file_validator{seqan3::output_file_open_options::open_or_create});

codecov · 2020-08-10T10:20:59Z

Codecov Report

Merging #2009 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2009   +/-   ##
=======================================
  Coverage   97.95%   97.96%           
=======================================
  Files         264      264           
  Lines       10045    10050    +5     
=======================================
+ Hits         9840     9845    +5     
  Misses        205      205

Impacted Files	Coverage Δ
include/seqan3/argument_parser/validators.hpp	`89.20% <100.00%> (+0.31%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 205674c...c94d251. Read the comment docs.

joergi-w

Thank you for this draft PR! I agree with the implementation and the tests are actually good (they cover all combinations of file formats and the boolean flag). I think after cleaning up the comments you can switch it to a "real" PR.

joergi-w · 2020-08-14T06:13:33Z

include/seqan3/argument_parser/validators.hpp

@@ -600,10 +600,16 @@ template <typename file_t = void>
 class output_file_validator : public file_validator_base
 {
 public:
+    //!\brief Stores, if it is valid to overwrite the output file.
+    bool allow_overwrite = false; // ToDo: bool oder enum? Für jedes File einzeln entscheiden, also zum Pfad dazugeben? Oder als Vektor angeben?


append cannot be implemented with the argument parser, as the user receives the file name and opens the file themself (with or without append flag). So I suggest to only focus on the binary distinction of the output file "may exist or not".

Note: If we would not handle it at all, the decision (of throwing an error if the file exists) could be still be made by the app developer after obtaining the filename.

include/seqan3/argument_parser/validators.hpp

joergi-w

Mostly documentation ideas...

doc/tutorial/argument_parser/index.md

CHANGELOG.md

include/seqan3/argument_parser/validators.hpp

joergi-w · 2020-08-28T11:54:44Z

include/seqan3/argument_parser/validators.hpp

@@ -575,6 +574,15 @@ class input_file_validator : public file_validator_base
    }
 };

+//!\brief Stores, if it is valid to overwrite the output file.
+enum output_file_open_mode


Question (no need to change, just an opinion):
How about naming it accept_existing_file?
Because mode expresses nothing useful about the function of this enum.

include/seqan3/argument_parser/validators.hpp

test/unit/argument_parser/format_parse_validators_test.cpp

joergi-w

Thanks! 👍

marehr

Waiting on naming

eseiler · 2020-08-31T09:35:31Z

https://doc.rust-lang.org/std/fs/struct.OpenOptions.html#implementations

include/seqan3/argument_parser/validators.hpp

marehr

first round :)

include/seqan3/argument_parser/validators.hpp

test/snippet/argument_parser/validators_output_file.cpp

test/unit/argument_parser/format_parse_validators_test.cpp

include/seqan3/argument_parser/validators.hpp

test/unit/argument_parser/format_parse_validators_test.cpp

marehr · 2020-09-08T08:32:00Z

Core-Meeting 2020-09-08

We want semi-regularity (that means default constructible) and as default case is conservative with seqan3::output_file_open_options::create_new.

marehr

I didn't go through all test cases. Thank you!

include/seqan3/argument_parser/validators.hpp

test/snippet/argument_parser/validators_output_file.cpp

test/unit/argument_parser/format_parse_validators_test.cpp

include/seqan3/argument_parser/validators.hpp

marehr

Sorry for being late; This is just some nail polish :)

include/seqan3/argument_parser/validators.hpp

test/snippet/argument_parser/validators_output_file.cpp

test/unit/argument_parser/format_parse_validators_test.cpp

…ermission [TEST] Update validator test [DOC] Update changelog, update (tutorial) documentation and code documentation Signed-off-by: Lydia Buntrock <[email protected]>

joergi-w · 2020-09-28T09:28:17Z

@seqan/core Can we still include this in the current 3.0.2 release or do we have to wait for the next one? I think many applications can already benefit from this.

marehr · 2020-09-28T09:49:03Z

@joergi-w We didn't plan it for this release and I think we shouldn't do last minute changes.

@Irallia We noticed that you put the changelog entry in the 3.0.2 section, can you create a new PR that moves that in the 3.0.3 section?

…gs for permission handling' PR (seqan#2009) Signed-off-by: Lydia Buntrock <[email protected]>

Irallia · 2020-09-28T14:15:18Z

@Irallia We noticed that you put the changelog entry in the 3.0.2 section, can you create a new PR that moves that in the 3.0.3 section?

Done in: #2177

Irallia self-assigned this Aug 6, 2020

Irallia force-pushed the feature/argument_parser/allow_overwrite_existing_files branch 3 times, most recently from 497ff94 to b4ad152 Compare August 7, 2020 14:16

joergi-w approved these changes Aug 14, 2020

View reviewed changes

Irallia marked this pull request as ready for review August 17, 2020 08:47

Irallia force-pushed the feature/argument_parser/allow_overwrite_existing_files branch 6 times, most recently from 71b2c62 to 21fa77e Compare August 26, 2020 10:59

Irallia requested a review from joergi-w August 28, 2020 09:21

joergi-w requested changes Aug 28, 2020

View reviewed changes

Irallia requested a review from joergi-w August 28, 2020 14:57

joergi-w approved these changes Aug 28, 2020

View reviewed changes

joergi-w requested review from a team and marehr and removed request for a team August 28, 2020 15:08

marehr reviewed Aug 30, 2020

View reviewed changes

Irallia force-pushed the feature/argument_parser/allow_overwrite_existing_files branch 2 times, most recently from 3fc5b0a to 85ee726 Compare August 31, 2020 10:45

Irallia requested a review from marehr August 31, 2020 11:01

marehr reviewed Sep 1, 2020

View reviewed changes

include/seqan3/argument_parser/validators.hpp Outdated Show resolved Hide resolved

marehr requested changes Sep 1, 2020

View reviewed changes

Irallia force-pushed the feature/argument_parser/allow_overwrite_existing_files branch from 85ee726 to 7632f75 Compare September 3, 2020 17:04

Irallia requested a review from marehr September 3, 2020 17:14

Irallia force-pushed the feature/argument_parser/allow_overwrite_existing_files branch 2 times, most recently from dda81ea to bed6158 Compare September 8, 2020 11:26

marehr requested changes Sep 8, 2020

View reviewed changes

Irallia requested a review from marehr September 8, 2020 19:48

marehr requested changes Sep 15, 2020

View reviewed changes

Irallia force-pushed the feature/argument_parser/allow_overwrite_existing_files branch from 171f5e5 to a85540b Compare September 22, 2020 13:39

[FEATURE] Use enum output_file_open_options to handle overwriting p…

c94d251

…ermission [TEST] Update validator test [DOC] Update changelog, update (tutorial) documentation and code documentation Signed-off-by: Lydia Buntrock <[email protected]>

Irallia force-pushed the feature/argument_parser/allow_overwrite_existing_files branch from a85540b to c94d251 Compare September 22, 2020 13:56

Irallia requested a review from marehr September 23, 2020 08:38

marehr approved these changes Sep 28, 2020

View reviewed changes

marehr merged commit bc5e30a into seqan:master Sep 28, 2020

eseiler mentioned this pull request Sep 28, 2020

3.0.2 Release #2001

Closed

27 tasks

Irallia added a commit to Irallia/seqan3 that referenced this pull request Sep 28, 2020

[DOC] fix changelog entry of 'Allow overwrite existing files, add fla…

523aff1

…gs for permission handling' PR (seqan#2009) Signed-off-by: Lydia Buntrock <[email protected]>

Irallia mentioned this pull request Sep 28, 2020

[DOC] fix changelog entry of 'Allow overwrite existing files...' #2177

Merged

eseiler mentioned this pull request Mar 15, 2022

out_file extensions doesn't work seqan/sharg-parser#63

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FEATURE] Allow overwrite existing files, add flags for permission handling #2009

[FEATURE] Allow overwrite existing files, add flags for permission handling #2009

Irallia commented Aug 6, 2020 •

edited by marehr

Loading

codecov bot commented Aug 10, 2020 •

edited

Loading

joergi-w left a comment

joergi-w Aug 14, 2020

joergi-w left a comment

joergi-w Aug 28, 2020

joergi-w left a comment

marehr left a comment

eseiler commented Aug 31, 2020 •

edited

Loading

marehr left a comment

marehr commented Sep 8, 2020 •

edited

Loading

marehr left a comment

marehr left a comment

joergi-w commented Sep 28, 2020

marehr commented Sep 28, 2020

Irallia commented Sep 28, 2020 •

edited

Loading

[FEATURE] Allow overwrite existing files, add flags for permission handling #2009

[FEATURE] Allow overwrite existing files, add flags for permission handling #2009

Conversation

Irallia commented Aug 6, 2020 • edited by marehr Loading

codecov bot commented Aug 10, 2020 • edited Loading

Codecov Report

joergi-w left a comment

Choose a reason for hiding this comment

joergi-w Aug 14, 2020

Choose a reason for hiding this comment

joergi-w left a comment

Choose a reason for hiding this comment

joergi-w Aug 28, 2020

Choose a reason for hiding this comment

joergi-w left a comment

Choose a reason for hiding this comment

marehr left a comment

Choose a reason for hiding this comment

eseiler commented Aug 31, 2020 • edited Loading

marehr left a comment

Choose a reason for hiding this comment

marehr commented Sep 8, 2020 • edited Loading

Core-Meeting 2020-09-08

marehr left a comment

Choose a reason for hiding this comment

marehr left a comment

Choose a reason for hiding this comment

joergi-w commented Sep 28, 2020

marehr commented Sep 28, 2020

Irallia commented Sep 28, 2020 • edited Loading

Irallia commented Aug 6, 2020 •

edited by marehr

Loading

codecov bot commented Aug 10, 2020 •

edited

Loading

eseiler commented Aug 31, 2020 •

edited

Loading

marehr commented Sep 8, 2020 •

edited

Loading

Irallia commented Sep 28, 2020 •

edited

Loading