Skip to content

Flaws in version 0.3.12 #80

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

Closed
bulletmark opened this issue Jul 12, 2022 · 12 comments
Closed

Flaws in version 0.3.12 #80

bulletmark opened this issue Jul 12, 2022 · 12 comments

Comments

@bulletmark
Copy link

I have been using leetcode-cli and it has been working well. Just installed version 0.3.12 and see immediate issues:

  1. Every time I run leetcode e 1 a new code directory is created in whatever directory I was previously in. Previously that directory was created wrt to the root setting. If I set the code path absolute then it ignores the ~ shortcut for users home directory which leetcode always respected previously.
  2. The file 1.two-sum.py gets created but it has C and C++ comments embedded all throughout (which looks incredibly ugly in my editor when trying to syntax highlight the file).
  3. An empty 1.two-sum.tests.dat file gets created.
  4. Documentation of changes is missing. What is @lc code=start/end about?
  5. README here says both edit and test functions are to Edit question by id.

There are probably more bugs since I have raised this in only the first 5 mins after installing 0.3.12.

Also, as a matter of process, it would be best to tag the repo with the version whenever a new crates.io release is made.

@clearloop
Copy link
Owner

Thanks for the report! just yanked v0.3.12, guess these are from #77

@wendajiang
Copy link
Collaborator

#77 maybe I should make this as feature that can be open or close in the config file ?

@wendajiang
Copy link
Collaborator

I will test in py environment in this weekend and try to fix this issue.

@bulletmark
Copy link
Author

@wendajiang step 1 is to document in the README what that means. Also, those marker lines are perhaps a good idea (I sometimes want to include code for local testing but not for submission) but at least add those marker lines with a leading comment appropriate to the language (i.e. Python in my example).

@bulletmark
Copy link
Author

Here's some more things to fix if you guys are keen. (Rewrite it in Python and I will submit PRs! ;) )

Error messages are ungraceful. E.g. if I run on a new system I get:

$  leetcode e 1
thread 'main' panicked at 'Get Password failed: NoEntry', src/plugins/chrome.rs:77:36
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Can't this (and other error messages) be made nicer? E.g. In the above case leetcode has created a new directory and toml file. Best it should say that, and also whenever csrf and session keys are empty then nicely suggest to the user that he needs to edit them?

The configuration file is at ~/.leetcode but it should be under ~/.config/leetcode/ [or ~/.config/leetcode.toml] as per standard Linux XDG file locations spec. Cached data (e.g. the Problems file) should be stored at ~/.cache/leetcode/.

@wendajiang
Copy link
Collaborator

Here's some more things to fix if you guys are keen. (Rewrite it in Python and I will submit PRs! ;) )

Error messages are ungraceful. E.g. if I run on a new system I get:

$  leetcode e 1
thread 'main' panicked at 'Get Password failed: NoEntry', src/plugins/chrome.rs:77:36
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Can't this (and other error messages) be made nicer? E.g. In the above case leetcode has created a new directory and toml file. Best it should say that, and also whenever csrf and session keys are empty then nicely suggest to the user that he needs to edit them?

The configuration file is at ~/.leetcode but it should be under ~/.config/leetcode/ [or ~/.config/leetcode.toml] as per standard Linux XDG file locations spec. Cached data (e.g. the Problems file) should be stored at ~/.cache/leetcode/.

The configuration file location is hard code in the code currently, and vscode plugin use the path ~/.config/leetcode if I'm not misremember, so maybe ~/.leetcode not introduce the conflict.

And the code path in the newest config file should be absolute dir, as I do not want save the code file in the root dir (the cache file), and the test file path is same as code path. FYI

@bulletmark
Copy link
Author

bulletmark commented Jul 12, 2022

The configuration file location is hard code in the code currently, and vscode plugin use the path ~/.config/leetcode if I'm not misremember, so maybe ~/.leetcode not introduce the conflict.

Well then perhaps the dir should be leetcode-cli. Still best to put the config and cache files in the correct places. Placing directly under home dir is legacy and not expected for modern software.

And the code path in the newest config file should be absolute dir, as I do not want save the code file in the root dir (the cache file), and the test file path is same as code path.

That breaks compatibility for existing users because just setting the code value to code used to work. It is better to be relative anyhow - why make the user specify the same root dir twice in his config? Also tilde (~) expansion used to work and that has been broken as well.

@bulletmark
Copy link
Author

Actually just realised what you mean about cache file. The code dir is redundant if you move the Problems file to it's proper location under ~/.cache/leetcode/ because there are no other files or dirs. Then just make the following defaults.

File: ~/.config/leetcode.toml:

[storage]
code = "~/leetcode"

If you think user's really may want to move their cache then add default cache = "~/.cache/leetcode" so they can change that.

@bulletmark
Copy link
Author

Just realized there may be a scripts dir. So perhaps:

[storage]
root = "~/leetcode"
code = "code"
scripts = "scripts"
cache = "~/.cache/leetcode"

@wendajiang
Copy link
Collaborator

Just realized there may be a scripts dir. So perhaps:


[storage]

root = "~/leetcode"

code = "code"

scripts = "scripts"

cache = "~/.cache/leetcode"

Yeah, to me, it's ok, this format can be backward compatibility.

@bulletmark
Copy link
Author

Another bug you could fix: It seems you have added a -t option to write a test file. I agree that the test file should not be written automatically as before, because users often don't need it at all. Assuming you fix the current bug (that an empty file gets written anyhow, even if -t is not specified), then you should also fix the issue that if a user has an existing solution file then -t is ignored even if no test file exists. You have to move your solution file away, run with -t, and then move your solution file back. A test file should always been written when -t is specified (even if it overwrites because you assume the user knows what he is doing). Actually, perhaps e -t should just write the test file and not even open the editor at all?

@wendajiang wendajiang mentioned this issue Jul 12, 2022
@wendajiang
Copy link
Collaborator

Close this issue, if it's necessary, it can be reopen.

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

No branches or pull requests

3 participants