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

Readline improvements #1292

Closed
wants to merge 1 commit into from
Closed

Conversation

stinos
Copy link
Contributor

@stinos stinos commented May 28, 2015

As per discussion in #1286, here are a couple of changes for readline

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) to 94.14% when pulling 379a06c on stinos:readline-improvements into a16715a on micropython:master.

@pfalcon
Copy link
Contributor

pfalcon commented May 30, 2015

msvc: Implement the mp_hal_xxx functions and enable mp-readline

I don't see anything specific to msvc there, please use port name as a prefix: "windows:".

windows/msvc/mphal.c

Please have it as windows/mphal.c per above

windows/msvc/mphal.c
Copyright (c) 2015 Damien P. George

Please use your copyright.

@pfalcon
Copy link
Contributor

pfalcon commented May 30, 2015

All commits but the last, for which issues were raised, are cherry-picked into master. Please rebase and fix them. Thanks.

@stinos
Copy link
Contributor Author

stinos commented Jun 1, 2015

windows/Makefile already used unix_mphal.c so I kept this one seperate initially.. Changed that now: it works and doesn't require termios which doesn't seem to be readily available for mingw. If someone really wants that it's easy to add a config option to switch between winapi and termios (which probably would be implemented using winapi anyway)

Please use your copyright.

Is there a compelling reason to do so? Also wouldn't it make sense then to do it for every file I created in the repository?

@dpgeorge
Copy link
Member

dpgeorge commented Jun 1, 2015

Is there a compelling reason to do so?

If the bulk of the file is your original work then you own it and it is your privilege to distribute/make available the file as you see fit. As part of contributing to this repo you agree to make things available under an MIT license, hence the MIT header with the copyright holder (you). If you copy an existing file from the repo and make changes to it then that file holds the original license and so should have the original copyright holder listed. In that case you can also list your name as an additional copyright holder for the changes/additions.

If you put my name one code that you wrote 100% then there is an agreement that you have given me the code and I'm free to do with it as I please (include releasing it under a different license).

Most importantly though we want to give credit where it's due :)

}
const int ret = esc_seq_process_vk(rec.Event.KeyEvent.wVirtualKeyCode);
if (ret) {
DEBUG_printf("vk");
Copy link
Contributor

Choose a reason for hiding this comment

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

It can't really work as expected with this here, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can since I have DEBUG_printf print to the debugger instead of the commandline. But yes it's obviously an oversight of mine, fixed.

@pfalcon
Copy link
Contributor

pfalcon commented Jun 2, 2015

Bad news is that this doesn't work under wine (cursor doesn't move, even for backspace).

@pfalcon
Copy link
Contributor

pfalcon commented Jun 2, 2015

Ok, the issue is described here: https://www.winehq.org/docs/wineusr-guide/cui-programs . Summary: "winconsole" should be used to run such apps, and here on Ubuntu. winconsole --backend=curses clearly looks better (--backend=user appears to be default in ubuntu, despite what's written on that page).

So, please fix DEBUG_printf() issue, then I'll merge it and add the above to README.

@stinos
Copy link
Contributor Author

stinos commented Jun 4, 2015

If you put my name one code that you wrote 100% then there is an agreement that you have given me the code and I'm free to do with it as I please (include releasing it under a different license).

Well that is fine by me - except the part about releasing it under a different possibly more restricting license though I don't immediately see you doing that :] I'll see if I can figure out which files I created etc.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.0%) to 94.1% when pulling 51fcfe4 on stinos:readline-improvements into 4d9cad1 on micropython:master.

@pfalcon
Copy link
Contributor

pfalcon commented Jun 4, 2015

Merged, thanks.

@pfalcon pfalcon closed this Jun 4, 2015
@dpgeorge
Copy link
Member

dpgeorge commented Jun 4, 2015

Thanks!

@stinos stinos deleted the readline-improvements branch June 22, 2015 09:13
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.

None yet

4 participants