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

feat: cli improvement #55

Merged
merged 2 commits into from Jul 5, 2023
Merged

feat: cli improvement #55

merged 2 commits into from Jul 5, 2023

Conversation

DeleMike
Copy link
Contributor

@DeleMike DeleMike commented Jul 2, 2023

Closes #47
I have done some research and added --dev/--no-dev and --build/--no-build instead of the former commands. This new option style allows the build file to now has python3 starfyre --build --path="test-application". It makes the command leaner.

If the user runs the build file, it will successfully build and run the required files to create our application.

Things Observed:
If I change the build file from python3 starfyre --build --path="test-application" to python3 starfyre --build --path="my-first-app". It will report the following error:

 File "/usr/local/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/local/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/home/akin/software_eng/starfyre/starfyre/__main__.py", line 74, in <module>
    main()
  File "/home/akin/software_eng/starfyre/.venv/lib/python3.10/site-packages/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
  File "/home/akin/software_eng/starfyre/.venv/lib/python3.10/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
  File "/home/akin/software_eng/starfyre/.venv/lib/python3.10/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/akin/software_eng/starfyre/.venv/lib/python3.10/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "/home/akin/software_eng/starfyre/starfyre/__main__.py", line 61, in main
    compile(path)
  File "/home/akin/software_eng/starfyre/starfyre/compiler.py", line 160, in compile
    build_dir.mkdir(exist_ok=True)
  File "/usr/local/lib/python3.10/pathlib.py", line 1175, in mkdir
    self._accessor.mkdir(self, mode)
FileNotFoundError: [Errno 2] No such file or directory: '/home/akin/software_eng/starfyre/my-first-app/build'

Is this an expected behaviour?

Is the --path="test_application" in the command contained in the starfyre build file constant or path value can change?

Copy link
Member

@sansyrox sansyrox left a comment

Choose a reason for hiding this comment

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

Hey @DeleMike 👋

Great work! I love the speed of PR 🔥

I have a few comments on the PR. Do let me know if you require any more information from me. 😄

starfyre/__main__.py Outdated Show resolved Hide resolved
starfyre/__main__.py Outdated Show resolved Hide resolved
build.sh Outdated Show resolved Hide resolved
@sansyrox
Copy link
Member

sansyrox commented Jul 2, 2023

Also, answering your question here.

If I change the build file from python3 starfyre --build --path="test-application" to python3 starfyre --build --path="my-first-app". It will report the following error:

It is because it will either expect a relative path or an absolute path. So, it is unable to find the folder as you do not have any folder named my-first-app in your starfyre directory.

You can decipher it from the error message here

FileNotFoundError: [Errno 2] No such file or directory: '/home/akin/software_eng/starfyre/my-first-app/build'

So, it is not a fixed value but this is the expected error.

@DeleMike
Copy link
Contributor Author

DeleMike commented Jul 3, 2023

Hi @sansyrox, I have updated the implementation - I made some changes with your suggestions, thanks! The issue now is this command python3 starfyre --path="my-first-app" works fine but I think it doesn't generate all the files needed for a successful run, right?

I compared it against the test_application and it is missing some files. How do these files get generated?

So because python3 starfyre --path="my-first-app" doesn't generate all the files needed, this command python3 starfyre --build --path="my-first-app" fails with this error:

Traceback (most recent call last):
  File "/usr/local/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/local/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/home/akin/software_eng/starfyre/starfyre/__main__.py", line 75, in <module>
    main()
  File "/home/akin/software_eng/starfyre/.venv/lib/python3.10/site-packages/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
  File "/home/akin/software_eng/starfyre/.venv/lib/python3.10/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
  File "/home/akin/software_eng/starfyre/.venv/lib/python3.10/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/akin/software_eng/starfyre/.venv/lib/python3.10/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "/home/akin/software_eng/starfyre/starfyre/__main__.py", line 66, in main
    subprocess.run(
  File "/usr/local/lib/python3.10/subprocess.py", line 503, in run
    with Popen(*popenargs, **kwargs) as process:
  File "/usr/local/lib/python3.10/subprocess.py", line 971, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/usr/local/lib/python3.10/subprocess.py", line 1847, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: '/home/akin/software_eng/starfyre/my-first-app/build/__init__.py'

I tried printing the absolute path and it is pointing to the right directory Absolute Path = /home/akin/software_eng/starfyre/my-first-app/build/__init__.py

Am I missing a step during compilation?

@DeleMike
Copy link
Contributor Author

DeleMike commented Jul 3, 2023

I mistakenly deleted the whole project while testing if the compilation was working but I was able to get the files back later.
I have cut out the section of code I was working with in the __main__.py file

@click.command()
@click.option("--path", default=".", help="Path to the project")
@click.option("--build", is_flag=True, help="Compile and build package")
def main(path, build):
    """
    Command-line interface to compile and build a project.
    Args:
        path (str): Path to the project directory.\n
        build (bool): Whether to start the build package.
    """
    # compile and build project
    temp_path = path
    path_ = temp_path + "/__init__.py"
    # get absolute path
    path = os.path.abspath(path_)
    compile(path)
    create_main_file(os.path.dirname(path))
    if build:
    # start/run project
        path_ = temp_path + "/build/__init__.py"
        path = os.path.abspath(path_)
        subprocess.run(
            [sys.executable, "-m", "build"],
            cwd=path,
            stdout=subprocess.PIPE,
            stderr=subprocess.PIPE,
        )


if __name__ == "__main__":
    main()

@DeleMike
Copy link
Contributor Author

DeleMike commented Jul 3, 2023

Okay, so an update. I saw that the compile function expected an absolute path and I was passing a relative one - so the issue is not fixed. If you run python3 starfyre --build --path="my-first-app" or python3 starfyre --path="my-first-app", they will run successfully.

The only question I have now is the build method only produced this file structure:

    my-first-app/
        build/
            __main__.py
        dist/
            store.js

are there not meant to be more files?

Copy link
Member

@sansyrox sansyrox left a comment

Choose a reason for hiding this comment

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

Hey @DeleMike 👋

Thank you for addressing the changes. However, I have a few more comments on this PR. Do let me know if you require any more clarification 😄

.gitignore Outdated Show resolved Hide resolved
build.sh Outdated Show resolved Hide resolved
starfyre/__main__.py Outdated Show resolved Hide resolved
starfyre/__main__.py Outdated Show resolved Hide resolved
@sansyrox
Copy link
Member

sansyrox commented Jul 3, 2023

Okay, so an update. I saw that the compile function expected an absolute path and I was passing a relative one - so the issue is not fixed. If you run python3 starfyre --build --path="my-first-app" or python3 starfyre --path="my-first-app", they will run successfully.
The only question I have now is the build method only produced this file structure:
are there not meant to be more files?

@DeleMike , there should be more files, given that we have merged the dev and the build flags. The .fyre files are compiled to .py files.

@DeleMike
Copy link
Contributor Author

DeleMike commented Jul 3, 2023

@DeleMike , there should be more files, given that we have merged the dev and the build flags. The .fyre files are compiled to .py files.

@sansyrox Can you please check this:

 if build:
        # Start/run project
        build_dir = os.path.join(path, "build")
        subprocess.run(
            [sys.executable, "-m", "build"],
            cwd=build_dir,
            stdout=subprocess.PIPE,
            stderr=subprocess.PIPE,
        )
    else:
        # Compile and build project
        compile(os.path.join(path, "__init__.py"))
        create_main_file(path)

Does the above actually represent that we have merged the two flags? You suggested that I should not compile if --build is not passed. If I shouldn't compile what tips can you suggest to merge both commands? Or has it been done right?

starfyre/__main__.py Outdated Show resolved Hide resolved
starfyre/__main__.py Outdated Show resolved Hide resolved
starfyre/__main__.py Outdated Show resolved Hide resolved
Copy link
Member

@sansyrox sansyrox left a comment

Choose a reason for hiding this comment

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

Hi @DeleMike 👋

Great work. I have left a few suggestions.

Also, I will be pushing a commit to fix the large diffs soon 😄

@sansyrox sansyrox force-pushed the cli-improvement branch 3 times, most recently from 170be6c to f1eaaa1 Compare July 4, 2023 10:04
@DeleMike
Copy link
Contributor Author

DeleMike commented Jul 4, 2023

Hello @sansyrox , Thanks for your suggestions. I have used them to make updates.

@sansyrox
Copy link
Member

sansyrox commented Jul 4, 2023

Thanks @DeleMike 😄

I have made a few changes finally. And this should be good to merge. Great work! 🔥

@DeleMike
Copy link
Contributor Author

DeleMike commented Jul 4, 2023

Thanks @DeleMike 😄

I have made a few changes finally. And this should be good to merge. Great work! 🔥

Thank you. @sansyrox !

@sansyrox
Copy link
Member

sansyrox commented Jul 4, 2023

Actually @DeleMike , one update. The build script has stopped working.

If you delete the build and the dist folder. And then re start the ./build script. There is no index.html in the dist folder. There must be a bug somewhere. Can you please have a look ?

@DeleMike
Copy link
Contributor Author

DeleMike commented Jul 4, 2023

Thanks. I'm looking into it now...

starfyre/__main__.py Outdated Show resolved Hide resolved
Copy link
Member

@sansyrox sansyrox left a comment

Choose a reason for hiding this comment

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

LGTM! Great work @DeleMike ! 🔥 ❇️

@sansyrox sansyrox merged commit 07e0547 into sparckles:main Jul 5, 2023
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.

Make the CLI interface smarter
2 participants