Skip to content
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

Command line config file download util #622

Open
wants to merge 31 commits into
base: master
Choose a base branch
from

Conversation

CodeOhms
Copy link

Updated the TUI code to use less memory, and added a help menu. Just need to implement autoConfig() functionality. Also thinking of changing the name of the program and moving it under 'utils' directory.

Zac and others added 19 commits August 14, 2018 22:24
Now prints to terminal. Needs fix for downloading as config download …
…y to browse truncated strings using left and right arrow keys.
…enu needs to be implemented, to show the user which buttons to use to navigate this tool. Fixed a fatal mistake where local copy of ttyProgressDialog was being used again for config dl despite going out of scope.
@CodeOhms
Copy link
Author

CodeOhms commented Aug 9, 2019

This program is ready, when you are.

@matlo
Copy link
Owner

matlo commented Aug 9, 2019

Thanks!

Please fix the remaining compilation warnings: https://travis-ci.com/matlo/GIMX/builds/122616488

@CodeOhms
Copy link
Author

CodeOhms commented Aug 9, 2019

Sure

@CodeOhms
Copy link
Author

Okay, all compiler warnings have now vanished for my program. Enjoy!

Copy link
Owner

@matlo matlo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a quick review. Main issue is file handling. I'll try to make some shared code to help fixing this.
Please replace tab indentation with 4-space indentation.

Makedefs Outdated Show resolved Hide resolved
Makedefs Outdated Show resolved Hide resolved
fetchconfig/Makefile Outdated Show resolved Hide resolved
fetchconfig/configDownload.cpp Outdated Show resolved Hide resolved
fetchconfig/configDownload.cpp Outdated Show resolved Hide resolved
fetchconfig/include/parseArgs.h Outdated Show resolved Hide resolved
fetchconfig/include/easyCurses.h Outdated Show resolved Hide resolved
fetchconfig/include/configDownload.h Outdated Show resolved Hide resolved
fetchconfig/include/configDownload.h Outdated Show resolved Hide resolved
fetchconfig/include/configDownload.h Outdated Show resolved Hide resolved
@CodeOhms
Copy link
Author

Thanks for taking the time to give feedback - means a lot to a beginner! I'll start working on those changes. If you like, I could also pull all the code I copied directly from gimx-launcher into a shared library.

Makedefs Show resolved Hide resolved
Makedefs Show resolved Hide resolved
fetchconfig/parseArgs.cpp Outdated Show resolved Hide resolved
configDl->chooseConfigs();
}

int callOptions(struct option* longOptions, int optionCharacter, int optionIndex)
{
int result = 0;
int result = -1;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parseArgs now stops on first "non-flag" option.

@matlo
Copy link
Owner

matlo commented Aug 13, 2019

I tested the manual download. In my opinion page up and page down should be inverted. Page down is usually used to go forward, and page up backward.

@CodeOhms
Copy link
Author

Okay

@matlo
Copy link
Owner

matlo commented Aug 13, 2019

On Ubuntu with a terminal size of 46x13, app crashes when displaying page 10.

@CodeOhms
Copy link
Author

I believe this is a rounding error, there should have been 9 pages. While cleaning up my code I noticed how badly it deals with corner cases when lines overflow the screen width. In spare time I've been writing a new mapping system, which is a lot easier to use. It will be fixed soon. I apologise. :)

…e system for handling input text. This means it can handle almost any custom terminal size. Also removed accidentally included binary file.
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

Successfully merging this pull request may close these issues.

2 participants