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

Change PtDebugPrint to behave more like Py3 print. #649

Merged
merged 1 commit into from
May 9, 2020

Conversation

Hoikas
Copy link
Member

@Hoikas Hoikas commented May 6, 2020

This is to facilitate H-uru/moul-scripts#119.

PtDebugPrint() now outputs to a single line and accepts the keyword argument sep, which functions in the same manner as Python 3's sep keyword argument. The end keyword argument was not added. Known limitation: the end argument cannot be used to elide newlines.

I have also fixed a memory leak in which the repr of a PyObject was not decref'd.

if (value) {
if (PyInt_Check(value))
level = PyInt_AsLong(value);
else
break;
}

value = PyDict_GetItemString(kwargs, "sep");
Copy link
Member

Choose a reason for hiding this comment

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

Bonus points: add end as well :)

print('Things: ', end='')  # Elide newline
for thing in things:
    print(thing, end=', ')
print()   # Finish the line

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately, plStatusLog unconditionally treats each call to AddLine as at least one line. Fixing that is outside the scope of this PR, but I will add an end argument that allows terminating with an arbitrary string.

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough, but I'd rather not see end default to '\n' if we're going to end up writing two newlines (one from AddLine and one from the default end)... Maybe it would be better to just leave out the end parameter until it's needed (and then update the DebugPrint to use it properly at the same time)

Copy link
Member Author

Choose a reason for hiding this comment

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

plStatusLog only adds the trailing \n if one is not present. I decided to default end to \n on the off chance that some later work allows eliding newlines in plStatusLog 😄

Sources/Plasma/FeatureLib/pfPython/cyMiscGlue4.cpp Outdated Show resolved Hide resolved
Sources/Plasma/FeatureLib/pfPython/cyMiscGlue4.cpp Outdated Show resolved Hide resolved
PYTHON_RETURN_NONE;
} while (false);

// fell through to the type error case
PyErr_SetString(PyExc_TypeError, "PtDebugPrint expects a sequence of strings and an optional int");
PyErr_SetString(PyExc_TypeError, "PtDebugPrint expects a sequence of strings, an optional int, and an optional string");
Copy link
Member

Choose a reason for hiding this comment

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

kwargs are not "optional parameters", since they need to be explicitly keyed... Also, since we're str()ifying parameters (to match print), I feel like this message is misleading in multiple ways.

Copy link
Member Author

Choose a reason for hiding this comment

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

:trollface:

@Hoikas
Copy link
Member Author

Hoikas commented May 6, 2020

Updated to fix issues from review.

Sources/Plasma/FeatureLib/pfPython/cyMiscGlue4.cpp Outdated Show resolved Hide resolved
if (value) {
if (PyInt_Check(value))
level = PyInt_AsLong(value);
else
break;
}

value = PyDict_GetItemString(kwargs, "sep");
Copy link
Member

Choose a reason for hiding this comment

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

Fair enough, but I'd rather not see end default to '\n' if we're going to end up writing two newlines (one from AddLine and one from the default end)... Maybe it would be better to just leave out the end parameter until it's needed (and then update the DebugPrint to use it properly at the same time)


value = PyDict_GetItemString(kwargs, "sep");
if (value) {
if (PyString_CheckEx)
Copy link
Member

Choose a reason for hiding this comment

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

I think you're missing something here :trollface:

Copy link
Member Author

Choose a reason for hiding this comment

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

Just making sure you're paying attention :trollface:

@Hoikas Hoikas merged commit fda8333 into H-uru:master May 9, 2020
@Hoikas Hoikas deleted the pydebugprint branch May 9, 2020 00:36
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

2 participants