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

warning and error equivalents of postf #5682

Open
madskjeldgaard opened this issue Jan 3, 2022 · 14 comments
Open

warning and error equivalents of postf #5682

madskjeldgaard opened this issue Jan 3, 2022 · 14 comments
Labels
enhancement good first issue indicates issue tickets that are suitable for a new contributor

Comments

@madskjeldgaard
Copy link
Contributor

Motivation

The .postf is a neat shortcut for posting a formatted string.

But personally I see my self doing things like "% is not implemented yet".format("something").warn and the equivalent with .error all the time.

Description of Proposed Feature

Add the following methods to String:


// Equivalent to .postf
String.warnf(...args);
String.errorf(...args);

// Equivalent to postln and format combined
String.warnlnf(...args);
String.errorlnf(...args);

And, now that we are at it, a String.postlnf method equivalent to .postf but with a newline like .postln.

@elgiano
Copy link
Contributor

elgiano commented Feb 21, 2022

I like this :) I just think they should be called postfln, warnfln, warnln etc..

@elgiano elgiano added the good first issue indicates issue tickets that are suitable for a new contributor label Feb 21, 2022
@telephon
Copy link
Member

Personally, I have stopped using postf() and use .format().postln instead. This is easy to read and easily combined with all string methods in the class library.

@Nini12345678901
Copy link

Hi I am interested in contributing to this issue! Would you please be able to guide me as to how I can get started and where I can locate the necessary files for making contributions to this?

@dyfer
Copy link
Member

dyfer commented Sep 16, 2022

@Nini12345678901 Thank you for your interest. You can find information for contributors on our wiki, particularly pages under "Developer reference" section in the toolbar on the right. I hope you find these resources useful!

@Nini12345678901
Copy link

Thank you for providing the link to the project's wiki! I have been reading through the different sections including Developer Reference. I am still unsure as to how I can navigate to the supporting files for this particular issue that I can use to set up on my IDE. I would appreciate the guidance, thank you.

@dyfer
Copy link
Member

dyfer commented Sep 18, 2022

@Nini12345678901 I'm not sure I understand the scope of your question. If you ask about where in SC you can find the implementation of e.g. .postf, then that's in the String.sc for the sclang side. I'd need to look further to see where the c++ implementation is (the proposed changes could be implemented either way I guess).

When you're in SC, you can find classes (with file locations) implementing a given method if you press ctrl-i/cmd-i with the cursor at the method name.

Personally, I have stopped using postf() and use .format().postln instead. This is easy to read and easily combined with all string methods in the class library.

However, from the discussion above I'm not sure if we have a clear path forward. Are we in agreement that we want to introduce the new method(s)? And what should their names be?

@telephon
Copy link
Member

I am in favor of keeping what we have and just using format. This combines well with anything that takes a string, and there is no need for eaxtra documentation. Just my 2 cents.

@Nini12345678901
Copy link

@dyfer Thanks for the further information. If I'm understanding this properly, wouldn't .postf be more concise for usage in warns and errors while still comprehending the specific issue that is being addressed? Furthermore, I think in regards to names, something along the lines of "postFormat" would be suitable for naming the methods.

I was unable to find the proper location of the C++ implementation as I would like to work on making any changes in regards to this issue in C++. If you have been able to find where they are, I'd appreciate it!

@jamshark70
Copy link
Contributor

When adding methods, we need to consider:

  • How much functionality is gained? (We like to think about this part.)
  • How much effort will it take to maintain the new code going further? (We don't like to think about this part, but not thinking about this part causes problems for future developers.)

errorFormat and warnFormat would convert

"A string with %".format("placeholders").warn

into

"A string with %".warnFormat("placeholders")

45 (old) vs 44 (new) characters... the case for a large functional benefit is relatively weak.

On the other hand, if the methods are simply implemented as wrappers in sclang (warnFormat { |... args| ^this.format(*args).warn }), the maintenance cost is also low. (Don't forget also about documentation cost, and potential questions on the forum about "what is the difference between x and y?" which is one of the things that happens when you have too many ways to do the same thing.)

If the new methods are implemented in C++, I suspect that the maintenance and testing costs would be higher. Because the methods are so easily implemented as one-liners in sclang, I'd advise against complicating the effort.

On balance, I think the benefit is not significant enough -- I'm with telephon here.

@Nini12345678901
Copy link

@jamshark70 Thank you for your insight. I do see the points you have made as costs for implementing the new methods in C++ could potentially be higher. Is there a way I can still contribute to this issue that would increase efficiency or add beneficial value to the existing implementation?

@jamshark70
Copy link
Contributor

Is there a way I can still contribute to this issue that would increase efficiency or add beneficial value to the existing implementation?

I'll leave that for others to decide. If it's up to me, I doubt that it's so important to do. If others really want it, then I won't object (low gain, but also low cost = not much reason for, also not much reason against).

@gorenje
Copy link
Contributor

gorenje commented Dec 28, 2022

I would go further and remove .postf - seems to be confusing (or perhaps inconsistent?) to have one shortcut, i.e., for .format(..).post but not for the rest.

As alternative, anyone who needs this (or wants) could implement their own shortcuts as class extensions:

+ String {
    postlnf { |..args|
        ^this.format(..args).postln;
    }
    ...
}

(admittedly syntax wrong)

my 2 cents :)

@telephon
Copy link
Member

I remember that in the beginning, I used to use postf, but now I have abandoned it for format and postln. Then I don't have to rethink when I want to use a format string with an error, e.g., I simply combine format and Error. Compressing two methods into one requires you to learn more words, and often you have to make choices about the combined arguments to the two methods.

@madskjeldgaard I don't really understand where your need comes from – is there any pressing problem?

@madskjeldgaard
Copy link
Contributor Author

I remember that in the beginning, I used to use postf, but now I have abandoned it for format and postln. Then I don't have to rethink when I want to use a format string with an error, e.g., I simply combine format and Error. Compressing two methods into one requires you to learn more words, and often you have to make choices about the combined arguments to the two methods.

@madskjeldgaard I don't really understand where your need comes from – is there any pressing problem?

It's not a pressing problem at all. Just a convience method that I think would match postf. There are a lot of other things that are more important right now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement good first issue indicates issue tickets that are suitable for a new contributor
Projects
None yet
Development

No branches or pull requests

7 participants