clearloop / leetcode-cli

May the code be with you 👻
MIT License
317 stars 54 forks source link

fix issue 80 #81

Closed wendajiang closed 2 years ago

wendajiang commented 2 years ago

80

  1. use code.test config to open/close writing test file
  2. use code.comment_leading to comment the desc
  3. use code.comment_problem_desc to open/close the description about the problem
  4. use code.start_marker to mark the code begin
  5. use code.end_marker to mark the code end

But, this update need user to update the toml config, otherwise the program get panic error massage, I not get the perfect method to handle this. Do you have a good way to handle this? @bulletmark @clearloop

wendajiang commented 2 years ago
const LANGS = [
  {lang: 'bash',       ext: '.sh',         style: '#'},
  {lang: 'c',          ext: '.c',          style: 'c'},
  {lang: 'cpp',        ext: '.cpp',        style: 'c'},
  {lang: 'csharp',     ext: '.cs',         style: 'c'},
  {lang: 'golang',     ext: '.go',         style: 'c'},
  {lang: 'java',       ext: '.java',       style: 'c'},
  {lang: 'javascript', ext: '.js',         style: 'c'},
  {lang: 'kotlin',     ext: '.kt',         style: 'c'},
  {lang: 'mysql',      ext: '.sql',        style: '--'},
  {lang: 'php',        ext: '.php',        style: 'c'},
  {lang: 'python',     ext: '.py',         style: '#'},
  {lang: 'python3',    ext: '.py',         style: '#'},
  {lang: 'ruby',       ext: '.rb',         style: '#'},
  {lang: 'rust',       ext: '.rs',         style: 'c'},
  {lang: 'scala',      ext: '.scala',      style: 'c'},
  {lang: 'swift',      ext: '.swift',      style: 'c'},
  {lang: 'typescript', ext: '.ts',         style: 'c'}
];

As now I introduce the code.comment_leading to let user to judge the comment style, should we use the default config like this?

bulletmark commented 2 years ago

I just ran this but clearly something is wrong. I have not tried anything more.

$ ./leetcode  l
error: missing field `start_marker` for key `code` at line 56 column 1, Parse config file failed,
leetcode-cli has just generated a new leetcode.toml at ~/.leetcode/leetcode_tmp.toml,
the current one at ~/.leetcode/leetcode.tomlseems missing some keys, please compare to the
tmp one and add them up : )

$ ls -l ~/.leetcode
total 4
-rw-r--r-- 1 mark mark 2139 Jul 12 14:27 leetcode.toml

Also, please fix the English:

Parse config file failed. leetcode-cli has just generated a new configuration file at ~/.leetcode/leetcode_tmp.toml as the existing file ~/.leetcode/leetcode.toml is missing keys. Please compare the new file and add the missing keys.

Why can't you list out the missing keys in the error message?

wendajiang commented 2 years ago

I just ran this but clearly something is wrong. I have not tried anything more.

$ ./leetcode  l
error: missing field `start_marker` for key `code` at line 56 column 1, Parse config file failed,
leetcode-cli has just generated a new leetcode.toml at ~/.leetcode/leetcode_tmp.toml,
the current one at ~/.leetcode/leetcode.tomlseems missing some keys, please compare to the
tmp one and add them up : )

$ ls -l ~/.leetcode
total 4
-rw-r--r-- 1 mark mark 2139 Jul 12 14:27 leetcode.toml

Also, please fix the English:

Parse config file failed. leetcode-cli has just generated a new configuration file at ~/.leetcode/leetcode_tmp.toml as the existing file ~/.leetcode/leetcode.toml is missing keys. Please compare the new file and add the missing keys.

Why can't you list out the missing keys in the error message?

Yeah, as i say, I know this problem, so if you have some suggestions about User must update their leetcode.toml ?

Or, I can fix this generate a leetcode_tmp.toml file( it should a old bug)

bulletmark commented 2 years ago

The problem is that your error message says a ~/.leetcode/leetcode_tmp.toml file is created but (as I show above!) there is no such file. So when you say "you know the problem", is that what you are referring to?

wendajiang commented 2 years ago

The problem is that your error message says a ~/.leetcode/leetcode_tmp.toml file is created but (as I show above!) there is no such file. So when you say "you know the problem", is that what you are referring to?

I know is not be created and maybe an old bug. My question is if it is the best way to help user update their toml config.

bulletmark commented 2 years ago

I've never had leetcode say to me that it has generated a new leetcode.toml at ~/.leetcode/leetcode_tmp.toml. So that must be a new change and thus if the file is missing that must be a new bug.

wendajiang commented 2 years ago

Please compare the new file and add the missing keys.

The newest version of this branch can generate the leetcode_tmp.toml file, you can try it. And I think this PR is ready to merge. However, If you give more suggestion, I will do more work to improve the experience. @clearloop @bulletmark

bulletmark commented 2 years ago

That's better. It's a pity that the user had to change the "comment_leading" value every time he changes "lang".

  1. You have added a option to turn "comment_problem_desc" which is good but the user should also have a way to turn the start and end markers off so they are not added at all. I tried setting them to an empty string but that did not work.

  2. I thought your original idea to add a -t option to the edit command was a better idea than how it worked before but I see you got rid of that and gone back to the original approach, although you have given an option to disable test files. Why not simply give the user the option to add the file with edit -t? That is a much better approach then having to edit the config file to turn it on and off.

  3. The title of this PR is "Fix issue 80" but if you look at my issue #80 then you have not addressed items number 4 and 5.

  4. As a note to both you and @clearloop , the leetcode.toml file unfortunately contains constants (at the top) which are not relevant to the user and clutter/confuse the file (i.e. everything in [sys] and [sys.urls]). In fact if the user edits those then he will break things. So that stuff should not be in a user configuration file or directory. Embed them in/with the program.

bulletmark commented 2 years ago

Another issue:

You can't test or submit any of your old solutions (or new solutions) unless they have the @lc code=start/end markers present. Otherwise you just get an error. The logic should only filter the file only if the markers are present. Note that this fix must be independent of the fix above needed to enable/disable the inclusion of markers when the file is created.

bulletmark commented 2 years ago

And another issue:

I can't work out what pattern causes this but some times the lines before @lc code=start are not filtered out or are filtered out in a partial way because I get "Runtime Error" for test or submit commands. If I paste the solution to leetcode directly it works, or if I hand comment out all the lines before the marker then it works also. This works for some problems and not for others, even problems that have essentially the same code. Just can't work out what different pattern causes it.

It should be simple logic:

  1. If the solution file has a @lc code=start marker then discard that marker line and all previous lines when sending.
  2. If the solution file has a @lc code=end marker then discard that line and all following lines when sending.
  3. There may be one marker line only in the file (either one of above) but above 2 rules still apply. I.e. user may have a "start" marker and no "end" marker, or vice-versa but still should filter. By default "start" is at start of file and "end" is at end of file.
  4. There may be no marker lines and above 2 rules still apply (i.e. all lines are sent).
  5. Should be irrelevant what comment characters are used on the marker lines. Just look for the marker pattern anywhere on a line.
wendajiang commented 2 years ago

And another issue:

I can't work out what pattern causes this but some times the lines before @lc code=start are not filtered out or are filtered out in a partial way because I get "Runtime Error" for test or submit commands. If I paste the solution to leetcode directly it works, or if I hand comment out all the lines before the marker then it works also. This works for some problems and not for others, even problems that have essentially the same code. Just can't work out what different pattern causes it.

Do you give a failed exmpale?

It should be simple logic:

  1. If the solution file has a @lc code=start marker then discard that marker line and all previous lines when sending.
  2. If the solution file has a @lc code=end marker then discard that line and all following lines when sending.
  3. There may be one marker line only in the file (either one of above) but above 2 rules still apply. I.e. user may have a "start" marker and no "end" marker, or vice-versa but still should filter. By default "start" is at start of file and "end" is at end of file.
  4. There may be no marker lines and above 2 rules still apply (i.e. all lines are sent).
  5. Should be irrelevant what comment characters are used on the marker lines. Just look for the marker pattern anywhere on a line.

The rule is simple that the content betwteen the end offset of @lc code=start and the start offset of @lc code=end would be send for testing or submitting.

Maybe should add a config item turn on/off the marker feature, but I think start and end marker should be on or off meanwhile. So It's not possible there is only @lc code=start or @lc code=end marker.

wendajiang commented 2 years ago

That's better. It's a pity that the user had to change the "comment_leading" value every time he changes "lang".

  1. You have added a option to turn "comment_problem_desc" which is good but the user should also have a way to turn the start and end markers off so they are not added at all. I tried setting them to an empty string but that did not work.

Yeah, maybe should add a config item that control the marker feature. And I propose:

  1. edit_desc to control the desc about problem
  2. If can not find the start marker, send all content to test or submit
    1. I thought your original idea to add a -t option to the edit command was a better idea than how it worked before but I see you got rid of that and gone back to the original approach, although you have given an option to disable test files. Why not simply give the user the option to add the file with edit -t? That is a much better approach then having to edit the config file to turn it on and off.

About this, I think user maybe think every input the command should edit -t a little fussy, and this behavior should be fixed, so I add to the configuration file not a option in the command.

  1. The title of this PR is "Fix issue 80" but if you look at my issue Flaws in version 0.3.12 #80 then you have not addressed items number 4 and 5.
  2. As a note to both you and @clearloop , the leetcode.toml file unfortunately contains constants (at the top) which are not relevant to the user and clutter/confuse the file (i.e. everything in [sys] and [sys.urls]). In fact if the user edits those then he will break things. So that stuff should not be in a user configuration file or directory. Embed them in/with the program.

Maybe the last two question, @clearloop can do better response, after all I only propose some PR for this project.