Skip to content

Conversation

@cutecutecat
Copy link
Contributor

fix issue #466

<>:"/ | ?* is not allowed in file path of Windows(all) and Linux(only /) when created separated database files with sequence names. Therefore, these symbols should be substituted with others.

At file src/util/unpack.cpp, substitute these forbidden symbols with others.
As '|' is frequently appeared in sequence name as:
A0A348AT68|unreviewed|Fluorescent
We change '|' to '!':
A0A348AT68!unreviewed!Fluorescent
Other symbols are changed to '@':
W5UC41|unreviewed|Nidogen-1|taxID/7998->W5UC41!unreviewed!Nidogen-1!taxID@7998

Then everything is ok!

<>:"/ \| ?* is not allowed in file path of Windows(all) and Linux(only /) when created separated database files with sequence names. Therefore, these symbols should be substituted with others.
@cutecutecat cutecutecat changed the title Fix the forbidden symbols when using unpack Fix the forbidden symbols when using unpackdb Jul 2, 2021
@milot-mirdita
Copy link
Member

Thanks a lot for your contribution and finding about this edge case.

We try to avoid Modern C++ features, could you please implement this without auto, lambdas and std::replace. I think FileUtil might be an appropriate place for this function.

The SSTR(key) branch is always numeric and doesn't require a call to path_substitution.

@cutecutecat
Copy link
Contributor Author

Thanks a lot for your contribution and finding about this edge case.

We try to avoid Modern C++ features, could you please implement this without auto, lambdas and std::replace. I think FileUtil might be an appropriate place for this function.

The SSTR(key) branch is always numeric and doesn't require a call to path_substitution.

Thank you for your review. These C++ 11 features will be removed in a couple of days. I will push next request before this Friday.

@milot-mirdita milot-mirdita merged commit c663497 into soedinglab:master Jul 7, 2021
@milot-mirdita
Copy link
Member

Thanks a lot!

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.

2 participants