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

Introduce a Parser module stub #146

Draft
wants to merge 19 commits into
base: master
Choose a base branch
from
Draft

Introduce a Parser module stub #146

wants to merge 19 commits into from

Conversation

Glutexo
Copy link
Owner

@Glutexo Glutexo commented Aug 26, 2022

I created a dummy module to start working on the Parser component. It lists all files in the current working directory with no extension. In a directory that only contains files owned by Onigumo, such files are those fetched by the Downloader.

I created a dummy module to start working on the Parser component. It lists all files in the current working directory with no extension. In a directory that only contains files owned by Onigumo, such files are those fetched by the Downloader.
@Glutexo Glutexo requested a review from nappex August 26, 2022 16:19
@Glutexo Glutexo self-assigned this Aug 26, 2022
@Glutexo
Copy link
Owner Author

Glutexo commented Aug 26, 2022

I am creating the pull request as a draft because there are no tests yet. I think the file listing routine is worth being tested.

The only way to run the new code is to manually edit Onigumo.CLI and replace the call to Onigumo.Downloader with Onigumo.Parser. #145 solves this by allowing to specify a module name.

Because the existing executive module is in a broadly-named file onigumo.ex, I added the new code into that file. If this gets into master first, the patch renaming the file (#141) will turn into one splitting it into two.

Currently, the downloaded files have no extension. It is not as straightforward to get all such files instead of getting those that do. Path.wildcard(root_path <> ".raw") would quickly get all files ending with “.raw.” Adding the extension (as already suggested in the flowchart in readme) would simplify finding the correct files. As a side effect, it would become safer (although still not recommended) to use an existing non-empty working directory.

With the file population in place, I see this will become a standalone module, as already suggested in #90 (comment).

@Glutexo Glutexo linked an issue Sep 5, 2022 that may be closed by this pull request
@Glutexo Glutexo removed a link to an issue Sep 5, 2022
@Glutexo Glutexo mentioned this pull request Sep 5, 2022
@nappex
Copy link
Collaborator

nappex commented Apr 7, 2023

I d like to add Path.expand check examples for dir? hexdocs. Its seems that can be problem with ~ in path.

@Glutexo Glutexo added the enhancement New feature or request label Apr 7, 2023
lib/onigumo/parser.ex Outdated Show resolved Hide resolved
@Glutexo
Copy link
Owner Author

Glutexo commented Apr 7, 2023

Currently, the root path is the current working directory, emerging in Onigumo.CLI. This value is always a real path that doesn’t require expansion. However, if the path becomes user-specified, it will need expanding by Path.expand.

Thank you for pointing this out! I created an issue #182 to track the possibility of setting the path via CLI.

|> File.ls!()
|> Enum.reject(&File.dir?(&1))
|> Enum.reject(&String.contains?(&1, "."))
|> Enum.join("\n")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line seems to be unnecessary, we can collect filename one by one, we dont have to print all filenames as one output.

@Glutexo
Copy link
Owner Author

Glutexo commented May 20, 2023

Merged current master dc0b3fc.

@Glutexo
Copy link
Owner Author

Glutexo commented May 20, 2023

Thanks to pull request #147, downloaded files now have the .raw extension. This has simplified the process of loading downloaded files. Without the extension, it was more complicated.

@Glutexo
Copy link
Owner Author

Glutexo commented May 20, 2023

I’ve noticed that retrieving all files without an extension also includes the executable file onigumo. 😃

Glutexo added 2 commits May 20, 2023 19:11
Downloaded files now have .raw extesion, they are no longer without an
extension at all.
Used Enum.map to trigger IO.puts instead of joining and printing in a
single blob.
@Glutexo
Copy link
Owner Author

Glutexo commented May 20, 2023

I introduced two changes:

  • 🚚 Append .raw to the downloaded file name #147 changed the downloaded file names to have a .raw extension. That resulted in a simpler and less error-prone file listing mechanism.
  • IO.puts does not print everything at once using Enum.join, but rather prints every item by itself using Enum.map.

lib/onigumo/parser.ex Outdated Show resolved Hide resolved
lib/onigumo/parser.ex Outdated Show resolved Hide resolved
Replace tabs with spaces.
@Glutexo Glutexo added the MVP label May 10, 2024
@Glutexo Glutexo force-pushed the parser branch 2 times, most recently from 493e10f to 8d2185b Compare May 10, 2024 19:10
Merged with the current master:

* Implemented behavior for Onigumo.Parser.
* Added Parser to CLI and environment.
lib/onigumo/parser.ex Show resolved Hide resolved
lib/onigumo/parser.ex Show resolved Hide resolved
Currently it doesn’t test anything for real. It’s just a test file
with a single test to be filled in.
@Glutexo
Copy link
Owner Author

Glutexo commented May 25, 2024

In the light of #232, this module stub may look unnecessary. Yet it contains a draft of a file discovery logic, that can be re-used by the new Feeder module. Also, at this point, only tests are remaining. For the sake of continuity and code re-use, I’d finish this pull request as it is.

Glutexo added 7 commits May 25, 2024 17:13
The Onigumo.Component behavior requires main to return :ok.
Made the function public, so it is testable.
Test an empty directory. Made list_downloaded public for it to be testable.
Values are compared with ==, not with =. Fixing an unused variable warning.
The parametrization allows to test various combinations of expected
and unexpected files.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request MVP
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

2 participants