LinGo icon indicating copy to clipboard operation
LinGo copied to clipboard

Codebase cleanup

Open jaredmontoya opened this issue 1 year ago • 5 comments

  • Added .gitignore to avoid accidentally checking compiled bytecode in to the VCS in the future.
  • Moved images to the .github folder
  • Fixed package
  • Removed deprecation warnings
  • go mod tidy
  • Moved all non-source files outside of src directory
  • hanzi.json is now embedded into the binary at compile time
  • automatic windows compatibility

jaredmontoya avatar Feb 14 '24 17:02 jaredmontoya

I suggest implementing these changes too:

  • Use github projects for todo's instead of the ToDo folder
  • Unify *_linux and *_windows versions of files as much as possible taking essential differences between them into another singular file that has nothing except those differences(if you don't do that you will have to maintain two versions of the main file in the future and keep track of the changes between them manually)
  • Create a unified config file (I like https://toml.io/en/ format) (you can use https://github.com/spf13/viper config system)
  • Use correct Os folders for different purposes instead of using current working directory
    • Linux
      • /tmp/LinGo/audio
      • ~/.config/LinGo.toml (that's where interface_language = "en" goes)
      • -~/.local/share/LinGo/languages
    • Windows
      • %Temp%\LinGo\audio
      • %AppData%\LinGo.toml
      • %AppData%\LinGo\languages
  • I don't know if having dictionary files stored is important because you can always create a new one but if it is dictionary files can be saved to the usual location inside of the language folder and then copied to the current working directory to avoid forcing users to go to ~/.local/share If storing dictionary files is not important they can be just generated into the current working directory from where they can be imported into Anki and then deleted
  • Instead of using texts folder, detect all text files in the current working directory and recognize them as texts This will make this program CWD(Current Working Directory) independent and installable as an application just like ls/cd/git/etc...

Users will be able to just enter a folder with text files and execute the lingo command to start practicing I don't think command line apps should have capital letters in their binary name, this app should have an all lowercase name to be easily executable as a command and installable as a package. Ex: AUR packages, APT packages all have lowercase names

jaredmontoya avatar Feb 14 '24 20:02 jaredmontoya

I removed the main_windows.go file per your request and renamed main_linux.go back to main.go. main_windows.go file is the main.go file that was inside of the windowsCompatibility folder. the main_linux.go file is the file that was in src/main.go

Is terminalSize.go from the windowsCompatibility that I moved to src/terminalSize/terminalSize_windows.go directory still needed or is it also obsolete like the main.go file for windows?

jaredmontoya avatar Feb 22 '24 14:02 jaredmontoya

Read the following as a friendly suggestion from another developer. I do not want to look like I am trying to force those changes on your project against your will even if I think they will greatly improve it's codebase quality and if you decide that they do not fit your style of writing code even after I explain what they are and how they (in my opinion) improve code quality I won't insist on them and will remove my cosmetic changes to the main file that do not affect it's code from this pull request, I do not mean anything bad and just want to help the project that I have great interest in using to learn a language.

When I tried to make sure that linux and windows versions of the main file had as little differences as possible I targeted only comments because I didn't spend time trying to understand what changes in the main_windows.go file make this program work on windows and why it wouldn't work if main_linux.go was used instead.

Turns out that main_windows.go is obsolete and main_linux.go that is now renamed back to main.go already runs perfectly on windows. But does that mean that changes to main.go must be reverted?

I can revert the changes that I did to main.go (formerly main_linux.go) but those changes are cosmetic and do not affect it's functionality. (except the changes that add hanzi.json embedding functionality to both main_linux.go and main_windows.go)

Those changes target comments only, some examples of that are:

This comment is unnecessary and I removed it because you don't have to explain what library is used for what inside of your main file as it can be inferred from the code itself or looked up on the internet or explained in project documentation. It almost seems like you are preparing this code to be presented to someone who doesn't know how to code and if that's the case I understand why there are such comments. But if they are meant for other developers then there is no point to write them because they can understand the code itself and will need to read documentation anyway if they want to know how to use those libraries. On top of that those libraries are either well-known(bubbletea) or standard libraries, if someone doesn't know what bubbletea is they can go to it's github page and read about what it is and get a lot more information than from that comment that just says it's related to UI.

// This is the main file: when you will run the compiled executable the code inside the main function
// is what will run.

// Imports:

/*
1) fmt --> for printing, formatting exc.
2) io/ioutil and os --> working with files
3) path/filepath --> used to list the number of subdirectories and files inside directories.
4) strings --> used for some string methods used throughout the program
6) Bubbletea and lipgloss --> used for the interface.
*/


Another thing that I did is I changed single-line comment format to multiline comment format in function documentation because I felt like multiline comments are better for that.

You can see that in the code is that you have an emtpy line between the comment describing the function and the function definition which leads to golang LSP not picking up the comment as documentation for the function. Example:

// This function initializes the bubbletea model to boot the application; this is one of the "dirtiest" parts of the application
// in terms of code and it needs a serious refactor.

func initialModel() model {
	// Get the boot language from the bootLanguage.txt file by reading it.
	bootLang, _ := os.ReadFile("setup/bootLanguage.txt")

With multiline comments it is easier to expand the documentation if the function grows and gets more features while not having to track if you have a space between the end of the comment and the beginning of the function because the end of the comment is always */

/*
This function initializes the bubbletea model to boot the application; this is one of the "dirtiest" parts of the application
in terms of code and it needs a serious refactor.
*/
func initialModel() model {
	// Get the boot language from the bootLanguage.txt file by reading it.
	bootLang, _ := os.ReadFile("setup/bootLanguage.txt")

Here I replaced the comment with it's longer version from main_windows.go that seemed to better explain what is happening

-       // Style for the text to quit (this comment says that text quits, not the program. which is inaccurate)
+       // This is the style for the "quit text", i.e the text that tells us how to quit the program.
	quitTextStyle = lipgloss.NewStyle().Margin(1, 0, 2, 4)

Those are the type of changes that were made and I don't see why they need to be reverted, I would also run all code in this repository through some formatter to improve it's consistency but I decided not to so it's easier to audit already a bit overwhelming changes in this pull request.

jaredmontoya avatar Feb 22 '24 16:02 jaredmontoya

Ok it looks good now; I saw you removed the ioutil deprecated library (I planned to as well but never did it idk why lol), and the comments look much better now. I'll test it a bit and merge it.

hsnborn22 avatar Feb 24 '24 11:02 hsnborn22

Thank you. I think this project has great potential.

jaredmontoya avatar Feb 24 '24 19:02 jaredmontoya