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

Update CLI helpdoc formatting to allow indentation in code #130

Merged
merged 7 commits into from
Dec 19, 2024

Conversation

sugatoray
Copy link
Contributor

@sugatoray sugatoray commented Dec 18, 2024

Use textwrap.dedent() to allow indented cli-helpdoc in __main__.py file. The indentation increases readability, while textwrap.dedent helps maintain the same functionality without breaking code.

image

Use `textwrap.dedent()` to allow indented cli-helpdoc in `__main__.py` file. The indentation increases readability, while `textwrap.dedent` helps maintain the same functionality without breaking code.
@sugatoray
Copy link
Contributor Author

@sugatoray please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@microsoft-github-policy-service agree

@sugatoray sugatoray marked this pull request as ready for review December 18, 2024 19:54
@sugatoray
Copy link
Contributor Author

cc: @gagb

@gagb
Copy link
Contributor

gagb commented Dec 18, 2024

can you please run precommit?

@gagb
Copy link
Contributor

gagb commented Dec 18, 2024

Can also please share a screenshot of how different the output of markitdown --help looks before and after?

@sugatoray
Copy link
Contributor Author

sugatoray commented Dec 18, 2024

Hi @gagb,

  1. I have updated the code post running pre-commit.

  2. Here is the screenshot of markitdown --help

    • AFTER the update: (please refer to the version number printed)

      image
    • BEFORE the update:

      image

@sugatoray
Copy link
Contributor Author

@gagb: I have shared the screenshots. Thank you.

@gagb
Copy link
Contributor

gagb commented Dec 18, 2024

I am unfortunately having a hard time seeing a difference? What did I miss? @afourney

@sugatoray
Copy link
Contributor Author

sugatoray commented Dec 18, 2024

Hi @gagb

I am unfortunately having a hard time seeing a difference? What did I miss? @afourney

There should not be any difference b/w the helpdoc before and after the change. The image at the top shows what changes in the code. And my screenshots show that the cli-help is still unchanged.

💡 There was a button to update the branch with regard to the main branch to avoid any conflicts later on. I clicked on it. And that's why it requires now for another pre-commit and tests validation.

image

@gagb
Copy link
Contributor

gagb commented Dec 18, 2024

My bad, I now understand the dedent feature. Pretty cool!

@gagb gagb requested a review from afourney December 18, 2024 23:34
@sugatoray
Copy link
Contributor Author

sugatoray commented Dec 19, 2024

My bad, I now understand the dedent feature. Pretty cool!

@gagb No issues. Perhaps the title of the PR was a bit confusing.

The branch was not ready to merge despite the checks being passed. So, I updated the branch (from the main branch). If you could let the checks re-run, and then merge it, that will be helpful.

Please let me know if there is anything else necessary from my end. Thank you.

@afourney
Copy link
Member

I've not used this before. Neat. Tests are running, and I'll merge once done.

@gagb gagb merged commit 7147bef into microsoft:main Dec 19, 2024
3 checks passed
@sugatoray
Copy link
Contributor Author

@gagb, @afourney: Thank you for reviewing and merging.

@gagb
Copy link
Contributor

gagb commented Dec 20, 2024

@gagb, @afourney: Thank you for reviewing and merging.

Thank YOU for contributing!

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.

3 participants