add CPParser

[hritikchaudhary]

• I'm the package's author and/or maintainer.
• I have have read the docs.
• I have tagged a release with a semver version number.
• My package repo has a description and a README describing what it's for and how to use it.
• My package doesn't add context menu entries. *
• My package doesn't add key bindings. **
• Any commands are available via the command palette.
• Preferences and keybindings (if any) are listed in the menu and the command palette, and open in split view.
• If my package is a syntax it doesn't also add a color scheme. ***
• If my package is a syntax it is named after the language it supports (without suffixes like "syntax" or "highlighting").
• I use .gitattributes to exclude files from the package: images, test files, sublime-project/workspace.

My package is a competitive programming file name and testcase parser, which parse codeforces and codechef and creates a cpp file with existing code additionally it also creates a test file of parsed testcases which can be used in CPPFastOlympicCoding plugin.

There are no packages like it in Package Control.

My package is similar to GetCode, However it should still be added because:

• GetCode no longer works; it is unable to parse even the problem title.
• It provides additional functionality for parsing test cases, which GetCode doesn't offer.
• Development on GetCode ceased four years ago.
• The developer is unresponsive to issues regarding the package.

[packagecontrol-bot]

Automated testing result: WARNING

Repo link: CPParser
Results help

Packages added:
  - CPParser

Processing package "CPParser"
  - WARNING: '.no-sublime-package' is defined. Please verify that it is *really* necessary

[braver]

Could your package function as a replacement of GetCode? @Salil03 would you be ok with replacement of your package? Or could you maybe let @hritikchaudhary contribute to it?

[hritikchaudhary]

Could your package function as a replacement of GetCode? @Salil03 would you be ok with replacement of your package? Or could you maybe let @hritikchaudhary contribute to it?

Hi @braver , Thanks for the review..
No it is similar is one way that it also creates a file with problem name, but it can't function as replacement because:

• it's is specifically designed to parse input testcases from codeforces and codechef, file creation is just an added benefit.
• The created testcase file can also be used with CPPFastOlympicCoding plugin which then automatically run the code against the fetched testcases.
  That's why I'd like you to consider it as a different plugin, I'll be actively working on it to resolve bugs and support more websites.

Also can you please help me with the warning: it's my first sublime plugin and not sure what the warning means.

Thanks!

[braver]

Alright, sounds good. Let's continue with the review then:

• Is CPParser ideal as a name? My mind immediately goes "CPP" meaning "c++". “CP Parser” would already be better, you can use spaces in package names.
• The warning is because you have the .no-sublime-package in your repo, meaning your package won't be packaged/zipped, but simply checked out into the Packages directory. This is not ideal, and should only be done if the package contains files that wouldn't be accessible otherwise, like scripts. If that's not the case, it usually points to a design flaw in the package...
  • You seem to be taking your package dir as the default location for storing files. That's not correct, you should not treat that as a writable directory. You have to assume that it will be wiped and replaced when the user uninstalls or upgrades your package. Instead write to a subdirectory (specific to your package) of the User directory, the path for which you can get via the API.
• Your package comes with a default settings file. Therefore you don't need to handle the file not existing, it will always be there. Ie. there is no need for the load_settings() wrapper you made. Simply access the directly.
• Please use edit_settings instead of open_file to open settings. This way you can make the settings open in Split View (as has been the normal and expected behavior in Sublime Text for what feels like close to a decade now...).
• In your Main.sublime-menu please only provide the id's of the pre-existing entries you're adding yours to. Otherwise you're basically redefining the captions for those menu items.
• I presume you noticed the thing about keybindings when creating this PR. I'm not sure users need to have that command under a key binding. Your command should at the very least be in the command palette.

[hritikchaudhary]

@braver addressed review comments...

* Is CPParser ideal as a name? My mind immediately goes "CPP" meaning "c++". “CP Parser” would already be better, you can use spaces in package names.

Updated name to CP Parser

* The warning is because you have the `.no-sublime-package` in your repo, meaning your package won't be packaged/zipped, but simply checked out into the Packages directory. This is not ideal, and should only be done if the package contains files that wouldn't be accessible otherwise, like scripts. If that's not the case, it usually points to a design flaw in the package...
  * You seem to be taking your package dir as the default location for storing files. That's not correct, you should not treat that as a writable directory. You have to assume that it will be wiped and replaced when the user uninstalls or upgrades your package. Instead write to a subdirectory (specific to your package) of the User directory, the path for which you can get via the API.

Default path is now ~/Documents/CPParser/ user can add their own in settings.

* Your package comes with a default [settings file](https://github.com/hritikchaudhary/CPParser/blob/main/CPParser.sublime-settings). Therefore you don't need to handle the file not existing, it will always be there. Ie. there is no need for the `load_settings()` wrapper you made. Simply access the directly.

Updated to directly access settings

* Please use `edit_settings` instead of `open_file` to open settings. This way you can make the settings open in Split View (as has been the normal and expected behavior in Sublime Text for what feels like close to a decade now...).

Updated to use edit_settings instead of open_file to open settings

* In your [Main.sublime-menu](https://github.com/hritikchaudhary/CPParser/blob/main/Main.sublime-menu#L22) please only provide the id's of the pre-existing entries you're adding yours to. Otherwise you're basically redefining the captions for those menu items.

Corrected

* I presume you noticed the thing about keybindings when creating this PR. I'm not sure users _need_ to have that command under a key binding. Your command _should_ at the very least be in the command palette.

Removed Key bindings added command in command palette

Please let me know if anythings else is missing
Thanks again!

[braver]

Default path is now ~/Documents/CPParser/ user can add their own in settings.

That's not what I wanted to suggest. And definitely not compatible across systems. Sublime Text packages should store files to a location inside Packages/User. Take for instance this example from SnippetMaker: https://github.com/jugyo/SublimeSnippetMaker/blob/master/SnippetMaker.py#L27C13-L32C14

[hritikchaudhary]

Default path is now ~/Documents/CPParser/ user can add their own in settings.

That's not what I wanted to suggest. And definitely not compatible across systems. Sublime Text packages should store files to a location inside Packages/User. Take for instance this example from SnippetMaker: https://github.com/jugyo/SublimeSnippetMaker/blob/master/SnippetMaker.py#L27C13-L32C14

Hi @braver, apologies I misunderstood
I have updated the location to use os.path.join( sublime.packages_path(), 'User', 'CPParser', '<Website Name>' ) if no location is provided https://github.com/hritikchaudhary/CPParser/blob/main/cp_parser.py#L144-L164
Thanks for the review!

[braver]

• You might want to get rid of those print statements (line 151, 165).
• Your code around getting the directory paths has become quite confusing this way. Your settings file has a default of "" for CODEFORCES_DIR, but the getter specifies a default of ".", and then if that somehow returns an empty value the sublime.packages_path based path kicks in as a third kind of default. Instead, might be better to move the sublime.packages_path based path into the get_codeforces/codechef_dir getters as the default passed to settings.get. It probably works as intended right now, but I had to read it 3 times to be sure of what it actually does.

I'll let you handle that in your own time, we can ship it in the mean time.

[packagecontrol-bot]

Automated testing result: ERROR

Repo link: CP Parser
Results help

Packages added:
  - CP Parser

Processing package "CP Parser"
  - ERROR: Unhandled exception in 'check' routine
    - File: messages.json
    - Exception: expected str, bytes or os.PathLike object, not list

[packagecontrol-bot]

Automated testing result: ERROR

Repo link: CP Parser
Results help

Packages added:
  - CP Parser

Processing package "CP Parser"
  - ERROR: Unhandled exception in 'check' routine
    - File: messages.json
    - Exception: expected str, bytes or os.PathLike object, not list

[braver]

Hold on, your messages.json is incorrect. You can specify exactly one file for "install" (or any specific version). I don't understand what "open_settings" is supposed to do there?

[hritikchaudhary]

Hi @braver Thank you for the review, and apologie for the messages.json, I was testing something during development, I missed to revert it.
I have simplified all three paths and fixed messages.json and also removed the print from L151 and L165

[packagecontrol-bot]

Automated testing result: SUCCESS

Repo link: CP Parser

Packages added:
  - CP Parser

Processing package "CP Parser"
  - All checks passed