Flaws in version 0.3.12

[bulletmark]

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]

Thanks for the report! just yanked v0.3.12, guess these are from https://github.com/clearloop/leetcode-cli/pull/77

[wendajiang]

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

[wendajiang]

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

[bulletmark]

@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]

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]

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]

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]

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]

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

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

[wendajiang]

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]

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]

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