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

Show stderr in the debugger #112

Closed
wants to merge 4 commits into from

Conversation

LitoMore
Copy link
Collaborator

@LitoMore LitoMore commented Sep 3, 2019

Resolve #86


IssueHunt Summary

Referenced issues

This pull request has been submitted to:


IssueHunt has been backed by the following sponsors. Become a sponsor

test/test.js Outdated
@@ -11,17 +10,6 @@ test('default', t => {
t.is(typeof alfyInstance.icon.error, 'string');
});

test.serial('.error()', async t => {
Copy link
Owner

Choose a reason for hiding this comment

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

Why are you removing the test? We still want to test .error().

@sindresorhus sindresorhus changed the title Show stderr in debugger Show stderr in the debugger Sep 23, 2019
@sindresorhus
Copy link
Owner

Bump

@LitoMore
Copy link
Collaborator Author

LitoMore commented Nov 1, 2019

Oh sorry, I missed this notification. I forgot why I removed this test. Let me check it.

@LitoMore
Copy link
Collaborator Author

LitoMore commented Nov 1, 2019

Now I'm facing another bug. It seems Script Filter has the length limitation.

For example:

alfy.output([{
	title: 'e'.repeat(1000),
	subtitle: 'Press ⌘L to see the full error and ⌘C to copy it.',
	valid: false,
	text: {
		copy: 'copy',
		largetype: 'stack'
	},
	icon: {
		path: exports.icon.error
	}
}]);

It will throw a JSON error like this:

[14:23:21.845] ERROR: alfred[Script Filter] JSON error: Unterminated string around character 30. in JSON:
{
	"items": [
		{
			"title": "eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee

We put too much letters to title, copy and largetype, they need to be simplified.

Should I do the simplify work in this pull request?

@sindresorhus
Copy link
Owner

Sure, what's the max length?

@LitoMore
Copy link
Collaborator Author

@sindresorhus The max length is 65532 (Alfred 4.0.6 [1124]).

@sindresorhus
Copy link
Owner

Truncate the title at 50000 then. Nobody will realistically have such long errors anyway.

@LitoMore
Copy link
Collaborator Author

Not only the title, copy and largetype also need to be truncated. 65532 is the max length of the whole JSON string.

@LitoMore
Copy link
Collaborator Author

Alfred will simplify the title like this below:

image

I think we could truncate the title at 100, truncate the copy and largetype at 20000 or smaller.

@sindresorhus
Copy link
Owner

65532 - 50000 = 15532, which is more than enough for the rest of the JSON, which is static.

@sindresorhus
Copy link
Owner

Ok, I now realize we're talking about different things. I was talking about the error specifically. You're talking about it generally.

title at 100 makes sense. largetype doesn't need that much. Let's cap it at 1000. The rest can be for the copy, which is the only thing that really needs to be long. We also need to documents these limits and why they are in place.

@sindresorhus
Copy link
Owner

Bump

@sindresorhus
Copy link
Owner

@LitoMore Bump

@LitoMore
Copy link
Collaborator Author

LitoMore commented Jul 7, 2020

Roger. I will follow up on the weekend.

@sindresorhus
Copy link
Owner

Bump

Base automatically changed from master to main January 23, 2021 07:48
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.

Alfy eats errors from Run Script action
2 participants