suffix() argument replace instead of append

[NiklasRosenstein]

An argument called replace rather than append for the suffix() function might be better suited and more self documenting.

[winksaville]

How about both?

On Sun, Sep 6, 2015, 2:51 PM Niklas Rosenstein notifications@github.com wrote:

An argument called replace rather than append for the suffix() function might be better suited and more self documenting.

— Reply to this email directly or view it on GitHub https://github.com/craftr-build/craftr/issues/12.

[NiklasRosenstein]

Both wouldn't make much sense, what would it do without an argument? And assuming replacing the suffix would be the default value:

def suffix(filename, text, replace=True, append=False):
    # ...

What would happen if you pass suffix(filename, 'o', append=True)? The function would have both replace and append set to True.

[winksaville]

I was assuming a named arg was always required. It would seem then the method should be called replaceSuffix otherwise the its unclear what it means, in fact it could be return the current suffix.

On Sun, Sep 6, 2015, 3:22 PM Niklas Rosenstein notifications@github.com wrote:

Both wouldn't make much sense, what would it do without an argument? And assuming replacing the suffix would be the default value:

def suffix(filename, text, replace=True, append=False):

...

What would happen if you pass suffix(filename, 'o', append=True)? The function would have both replace and append set to True.

— Reply to this email directly or view it on GitHub https://github.com/craftr-build/craftr/issues/12#issuecomment-138132634.

[NiklasRosenstein]

I was thinking about the action "to suffix" or "to prefix" something. But I guess using "replace_suffix" and "add_suffix" or "append_suffix" would suite better.

[winksaville]

I'd go with append/replace.

On Sun, Sep 6, 2015, 3:44 PM Niklas Rosenstein notifications@github.com wrote:

Reopened #12 https://github.com/craftr-build/craftr/issues/12.

— Reply to this email directly or view it on GitHub https://github.com/craftr-build/craftr/issues/12#event-402300502.

[NiklasRosenstein]

Or maybe with rmsuffix and addsuffix? Then you could do addsuffix(rmsuffix(files), 'o') instead of replacesuffix(files, 'o'). The reason why I think about doing this is we would need distinct functions in the platform modules as well, like add_obj() and replace_obj().

[NiklasRosenstein]

To be consistent, addprefix() would need to really add the text to the beginning of the string. addbaseprefix() would then do the job of prepending the text to the files basename.

[winksaville]

Personally I'd go with remove instead of rm, but if you like short names I'd go with rmv. As for replace do addnit if you find addXxx(rmvXxx) is common.

Another consideration is should authors of Craftrnfiles be required to specify the suffixes. I'd say know as its platform specific. So should be something done by implementors of compiler modules, correct?

On Sun, Sep 6, 2015, 3:57 PM Niklas Rosenstein notifications@github.com wrote:

To be consistent, addprefix() would need to really add the text to the beginning of the string. addbaseprefix() would then do the job of prepending the text to the files basename.

— Reply to this email directly or view it on GitHub https://github.com/craftr-build/craftr/issues/12#issuecomment-138134468.

[NiklasRosenstein]

I've settled with addprefix() being as it used to be, it will add a prefix to the base of the filename (as would make the most sense in most cases I can think of), addsuffix() now simply appends, but in case you're naughty you can still pass replace=True to instruct it to replace the suffix instead of append it. rmvsuffix() will remove all file suffixes.

[winksaville]

SG

On Sun, Sep 6, 2015, 5:10 PM Niklas Rosenstein notifications@github.com wrote:

Closed #12 https://github.com/craftr-build/craftr/issues/12 via 1a47c2b https://github.com/craftr-build/craftr/commit/1a47c2b46c8c6533ca5ed13b876cf116f08914af .

— Reply to this email directly or view it on GitHub https://github.com/craftr-build/craftr/issues/12#event-402318781.