ArrangePy icon indicating copy to clipboard operation
ArrangePy copied to clipboard

Added command line interface to Clean Py

Open kssvrk opened this issue 5 years ago • 4 comments

This is a PR against Issue 46

I have modified the sys argv methods and added CLI through 3 different arguments

  1. Required clean_dir variable to clean, throws OSERROR if wrong folder is passed.
  2. arrange modes of "strong" and "weak"
  3. nowarning flag.

Accordingly i modified the main file with choice variable and validation using exceptions. Did a little bit of code cleaning.

kssvrk avatar Dec 25 '20 06:12 kssvrk

Thanks @kssvrk !

There are a few things that I would like to change:

  1. If target directory is not provided, it should work on the current directory
  2. Default cleaning behaviour should be weak arrange and not strong
  3. I don't think --arrange flag should be even an option since that's the only thing that we do.

It should rather be like

  1. cleanpy -> weak arrange current directory
  2. cleanpy TARGET_DIR -> weak arrange TARGET_DIR
  3. cleanpy --strong TARGET_DIR -> strong arrange TARGET_DIR
  4. cleanpy --strong -nw -> strong arrange current directory with no warning

For the point 3 in the first paragraph, I would also ask how would we move ahead when we have large files feature integrated as well. A new flag --large-files? But I think that the default behavior of cleanpy should be arranging only

prashantsengar avatar Dec 25 '20 14:12 prashantsengar

I have a few reservations against the no default directory provided case.

Most of the time it defaults to CleanPy installation directory for cleaning. User can not cd into any other directory and execute cleanpy from that directory even if he uses full path of main.py because the CleanPy uses lib.utils without any setup in environment variables.

So it wouldn't work out. Any comments?

On Fri, 25 Dec 2020, 19:44 Prashant Sengar, [email protected] wrote:

Thanks @kssvrk https://github.com/kssvrk !

There are a few things that I would like to change:

  1. If target directory is not provided, it should work on the current directory
  2. Default cleaning behaviour should be weak arrange and not strong
  3. I don't think --arrange flag should be even an option since that's the only thing that we do.

It should rather be like

  1. cleanpy -> weak arrange current directory
  2. cleanpy TARGET_DIR -> weak arrange TARGET_DIR
  3. cleanpy --strong TARGET_DIR -> strong arrange TARGET_DIR
  4. cleanpy --strong -nw -> strong arrange current directory with no warning

For the point 3 in the first paragraph, I would also ask how would we move ahead when we have large files feature integrated as well. A new flag --large-files? But I think that the default behavior of cleanpy should be arranging only

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/prashantsengar/CleanPy/pull/51#issuecomment-751255249, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMCJYV2Y5Z7AMUSNMSRDQALSWSM3LANCNFSM4VI4CIJQ .

kssvrk avatar Dec 25 '20 14:12 kssvrk

But since we are packaging it for PyPI, most of the users would be installing it with pip and then use it via the command line. At that point, it would be safe to assume that most of the time the user runs it, the current directory is the directory where he/she wants to run it.

I hope I am getting it right.

Also this reminds me of configuration while running it via pip package. We would need an option to let the user change configuration from the CLI. I think I would have to do it while packaging it.

On Fri, Dec 25, 2020, 7:53 PM RADHA KRISHNA KAVULURU < [email protected]> wrote:

I have a few reservations against the no default directory provided case.

Most of the time it defaults to CleanPy installation directory for cleaning. User can not cd into any other directory and execute cleanpy from that directory even if he uses full path of main.py because the CleanPy uses lib.utils without any setup in environment variables.

So it wouldn't work out. Any comments?

On Fri, 25 Dec 2020, 19:44 Prashant Sengar, [email protected] wrote:

Thanks @kssvrk https://github.com/kssvrk !

There are a few things that I would like to change:

  1. If target directory is not provided, it should work on the current directory
  2. Default cleaning behaviour should be weak arrange and not strong
  3. I don't think --arrange flag should be even an option since that's the only thing that we do.

It should rather be like

  1. cleanpy -> weak arrange current directory
  2. cleanpy TARGET_DIR -> weak arrange TARGET_DIR
  3. cleanpy --strong TARGET_DIR -> strong arrange TARGET_DIR
  4. cleanpy --strong -nw -> strong arrange current directory with no warning

For the point 3 in the first paragraph, I would also ask how would we move ahead when we have large files feature integrated as well. A new flag --large-files? But I think that the default behavior of cleanpy should be arranging only

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub < https://github.com/prashantsengar/CleanPy/pull/51#issuecomment-751255249>, or unsubscribe < https://github.com/notifications/unsubscribe-auth/AMCJYV2Y5Z7AMUSNMSRDQALSWSM3LANCNFSM4VI4CIJQ

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/prashantsengar/CleanPy/pull/51#issuecomment-751256364, or unsubscribe https://github.com/notifications/unsubscribe-auth/AK43YGEEWVTXUHI4NR5OQ4LSWSN4ZANCNFSM4VI4CIJQ .

prashantsengar avatar Dec 25 '20 16:12 prashantsengar

Oh yes, So it would make sense then. So I'd have to take the current working directory. Ok I'd make the changes and inform.

It would look good as you said once you merge your PR of packaging.

kssvrk avatar Dec 25 '20 16:12 kssvrk