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

Changed the execution method to subcommand method #7

Merged
merged 1 commit into from
Feb 2, 2024
Merged

Conversation

noborus
Copy link
Owner

@noborus noborus commented Nov 29, 2023

Execute with query or list subcommand.
Changed the method to stop importer and add reader.
Changed the file name so that it can be used as a table.

Summary by CodeRabbit

  • New Features

    • Introduced a new list command for analyzing and outputting file contents.
    • Added a new query command for executing SQL-like queries on data.
  • Enhancements

    • Integrated file analysis and dumping logic into the queryExec function.
    • Streamlined the root command to display help when run without arguments.
  • Refactor

    • Created a new mdtsql package with a MDTReader type for improved data reading capabilities.
    • Consolidated text conversion functions within the mdtsql package.
  • Bug Fixes

    • Adjusted the ReadRow method logic to ensure accurate data processing.
  • Chores

    • Removed unused code and functions to clean up the codebase.

Execute with query or list subcommand.
Changed the method to stop importer and add reader.
Changed the file name so that it can be used as a table.
Copy link
Contributor

coderabbitai bot commented Nov 29, 2023

Walkthrough

The project has undergone a refactoring that integrates the analysis and dumping logic into the queryExec function, removing the need for a separate analyzeDump function. A new list command has been added, leveraging the mdtsql package for file analysis and output. The query command has been enhanced with additional logic for execution, and the root command now defaults to showing help. The mdtsql package introduces a new MDTReader for parsing and reading table data, streamlining the handling of markdown tables.

Changes

File(s) Summary
cmd/exec.go Removed analyzeDump function; integrated logic into queryExec.
cmd/list.go, cmd/query.go Added new commands list and query with associated logic and functions.
cmd/root.go Removed "log" import; updated Run function to show help.
reader.go Introduced mdtsql package with MDTReader type and functions for markdown table parsing.
table.go Updated ReadRow method logic; removed toText function.

Poem

🐇 CodeRabbit hopped along,
Refactoring code, making it strong.
Commands now sleek, with logic so neat,
Markdown tables, they now can read. 📄✨


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • If you reply to a review comment from CodeRabbit, the bot will automatically respond.
  • To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
  • Note: Review comments are made on code diffs or files, not on the PR overview.
  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 7

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 94449b8 and 308c4a0.
Files selected for processing (6)
  • cmd/exec.go (1 hunks)
  • cmd/list.go (1 hunks)
  • cmd/query.go (1 hunks)
  • cmd/root.go (2 hunks)
  • reader.go (1 hunks)
  • table.go (1 hunks)
Files skipped from review due to trivial changes (2)
  • cmd/root.go
  • table.go
Additional comments: 8
cmd/exec.go (1)
  • 16-16: The integration of mdtsql.MarkdownQuery within queryExec aligns with the refactor to streamline the execution flow. Ensure that all necessary functionality previously contained in analyzeDump is now properly handled by mdtsql.MarkdownQuery.
cmd/list.go (2)
  • 14-20: The Short and Long descriptions for listCmd appear to be placeholders. Please update them to accurately describe the list command's functionality.

  • 41-43: The init function correctly adds listCmd to rootCmd.

cmd/query.go (5)
  • 27-28: Verify where the Debug variable is defined and ensure it is accessible within the exec function's scope.

  • 32-32: Ensure that the newWriter function is implemented correctly and handles any potential errors.

  • 35-36: Verify that the Header variable is defined and correctly used, and consider explaining or defining the magic number 100 used for InPreRead to improve code readability.

  • 40-40: Ensure that the trd.Exec(query) function handles all necessary error conditions and provides informative error messages to the user.

  • 43-44: Check if queryCmd requires any flags or additional configuration that should be set up in the init function.

Comment on lines +32 to +38
func analyzeDump(fileName string, caption bool) error {
im, err := mdtsql.Analyze(fileName, caption)
if err != nil {
return err
}
im.Dump(os.Stdout)
return nil
Copy link
Contributor

Choose a reason for hiding this comment

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

The analyzeDump function should handle the case where fileName is an empty string before proceeding with the analysis.

Comment on lines +48 to +50
source, err := io.ReadAll(reader)
if err != nil {
return nil, err
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider streaming the input or processing it in chunks to handle large files more efficiently and avoid potential memory issues.

Comment on lines +109 to +133
func (r *MDTReader) parseNode(node ast.Node, target int) error {
switch node.Type() {
case ast.TypeDocument:
for n := node.FirstChild(); n != nil; n = n.NextSibling() {
if err := r.parseNode(n, target); err != nil {
return err
}
}
case ast.TypeBlock:
if node.Kind() == gast.KindTable {
r.tables = append(r.tables, node)
}

switch node.Kind() {
case ast.KindHeading, ast.KindParagraph:
if r.caption {
r.tableName = string(node.Text(r.source))
}
}
default:
fmt.Fprintf(os.Stderr, "unknown node:")
fmt.Fprintf(os.Stderr, "%v:%v\n", node.Kind(), node.Type())
}

return nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Be cautious of potential stack overflow due to deep recursion in parseNode. Consider using an iterative approach if the input document can be deeply nested.

}
t.body = append(t.body, data)
default:
return t, fmt.Errorf("unknown node %v:%v", n.Kind(), n.Type())
Copy link
Contributor

Choose a reason for hiding this comment

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

Improve error handling by providing more context in the error message returned from tableNode when encountering an unknown node.

Comment on lines +193 to +197
func toText(buf []byte) string {
if len(buf) > 0 {
return string(buf)
}
return ""
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure that toText correctly handles different text encodings. Consider using a library that can detect and convert character encodings to UTF-8.

Comment on lines +200 to +201
func init() {
trdsql.RegisterReaderFunc("MD", NewMDTReader)
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid potential conflicts by ensuring the key "MD" used in trdsql.RegisterReaderFunc is unique or configurable.

Comment on lines +14 to +20
Short: "A brief description of your command",
Long: `A longer description that spans multiple lines and likely contains examples
and usage of using your command. For example:

Cobra is a CLI library for Go that empowers applications.
This application is a tool to generate the needed files
to quickly create a Cobra application.`,
Copy link
Contributor

Choose a reason for hiding this comment

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

The Short and Long descriptions for queryCmd are placeholders and should be updated to accurately describe the command's functionality.

@noborus noborus merged commit 308c4a0 into master Feb 2, 2024
2 checks passed
@noborus noborus deleted the sub-command branch February 3, 2024 13:39
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.

1 participant