[Home]

Summary:ASTERISK-06833: [patch] Add a function to left/right justify text
Reporter:Terry Wilson (twilson)Labels:
Date Opened:2006-04-24 01:01:10Date Closed:2011-06-07 14:03:08
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Functions/func_strings
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) justify_diff.txt
( 1) justify_diff2.txt
Description:Here is a patch to add a dialplan function to left/right justify text with an optional padding character (default to right justification with the space character).  I wrote this as the first step to writing a modified version of custom cdrs for interoperating with various external serial-based systems (like hotel Property Management Systems).  For instance, the HOBIC standard requires a right justified 0-filled sequence number, and a left justified space filled extension.  With a cdr module that works like the cdr_custom module, you could achieve the proper textual format with these functions.

****** STEPS TO REPRODUCE ******

synopsis = "Left or right (default) justify the string padded with an optional characater (default ' ')."
syntax = "JUSTIFY(<string>|<length>|<right|left>|<padstring>)"
Example: ${JUSTIFY(hello,7,right,X) should return XXhello
Comments:By: Jason Parker (jparker) 2006-05-03 20:52:36

I personally like the idea behind this feature.  I do see a few small things that could/should be improved though.

1) You have a small formatting violation at the end of AST_DECLARE_APP_ARGS, you have 3 spaces, then );
2) Since padchar and direction are optional, you should make it something more like - JUSTIFY(<string>|<length>[|<right|left>[|<padchar>]])
3) If possible, you should use something like ast_build_string instead of strncat
4) Why are you using args.string in the strncasecmp with args.direction?
By: Tilghman Lesher (tilghman) 2006-05-04 00:06:07

I created bugASTERISK-6892 which should be able to do these types of padding formats, plus a whole lot more.
By: Jason Parker (jparker) 2006-05-04 00:58:35

I'm going to have to agree with Corydon76 here.

twilson, do you agree that this should be closed now?  If, for some reason, 7078 doesn't make it in, we can reopen this.
By: Terry Wilson (twilson) 2006-05-04 17:17:10

As long as there is something in there that will support the functionality, I'll be happy.  The sprintf function could work for me... might be a bit complex to use for a non-programmer if all they are wanting to do is justify some text.  Perhaps if some example documentation went in the usage for the padding cases?  Also, can sprintf pad with characters other than ' ' and '0'?

1) oops, thanks. Fixed
2) see 1)
3) good idea, thanks.  Fixed.
4) ah, well, um... yeah.  Fixed.  ;-)
By: Jason Parker (jparker) 2006-05-04 17:54:32

Corydon76, can you provide 2 examples (1 using left padding, and 1 using right padding), where 'X' is used as the pad character, using SPRINTF?

If so, I think we can close this.  Deprecating code before it even gets committed sucks, but...sometimes, it happens.  Don't worry, I'll still give you positive karma. :p
By: Tilghman Lesher (tilghman) 2006-05-04 18:02:59

You're correct.  The SPRINTF function only pads with either 0 or space.
By: Jason Parker (jparker) 2006-05-04 19:36:21

twilson, I was asked the question "Why would anybody want to pad a string with anything besides 0 or ''?".

I'll leave that question for you to answer.  Do you feel we should close this bug and work with SPRINTF instead?
By: Leif Madsen (lmadsen) 2006-05-04 20:38:14

While I can't think of a good reason why you would want to pad with something else, I also can't think of a good reason why you would want to limit the functionality. Yes, this is an ambiguous message :)
By: Jason Parker (jparker) 2006-05-04 21:51:31

It's the thought that counts, blitzrage :p
By: Tilghman Lesher (tilghman) 2006-05-04 22:05:50

blitzrage:  while SPRINTF does far more than just padding strings, the question is, does its lack of ability to pad characters other than zero or space make any difference?  I don't know that I've ever padded a field with anything other than space or zero, but my experience is not the end-all-be-all of needed padding, either.
By: Terry Wilson (twilson) 2006-05-05 11:16:54

The original reason that I wanted this functionality is for use with CDR records for interoperability with external systems that require fixed width fields.  The one standard that I am currently working on doesn't require anything but spaces or zeroes.  I'm not aware of anything that uses anything else, but I didn't feel comfortable leaving it out becuase I don't know everything (and people have a tendency to find ways to use what you write that you hadn't thought of).  So for the case I need it, the SPRINTF works fine for me and I certainly won't complain if the decision is made to go with Corydon's patch (great patch, btw.) as it is more generally useful.  The only reason that I would argue for the inclusion of this is on the "off chance" that someone might find it useful--which certainly wouldn't be a compelling argument if one had to choose between JUSTIFY and SPRINTF (I would choose SPRINTF in that case)--but might be just compelling enough since we don't actually have to choose.  Your choice, I'm open to either since both solve *my* need.  And that's what we all should really worry about!  ;-)
By: Jason Parker (jparker) 2006-05-05 11:42:33

Let's go ahead and close this then.  If anybody has the need for the specific functionality that this provides (which is NOT provided by SPRINTF), please contact a bug marshal to reopen this bug, and we'll reexamine it at that time.