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

New TextPrefix function #3160

Merged
merged 1 commit into from Feb 23, 2024
Merged

New TextPrefix function #3160

merged 1 commit into from Feb 23, 2024

Conversation

jsmeix
Copy link
Member

@jsmeix jsmeix commented Feb 20, 2024

  • Type: Enhancement

  • Impact: Normal

  • Description of the changes in this pull request:

New TextPrefix function to
remove leading space and tab characters from input lines
and prefix each line with the argument string if specified.

The intent is to be able to properly indent multi-line message strings
in the code and output the message without the code indentation
but prefixed as needed for example like

    message="first line
             second line
             last line"
    LogPrint "Message text:$LF$( TextPrefix ' | ' <<<"$message" )"

which results the following output

Message text:
 | first line
 | second line
 | last line

@jsmeix jsmeix added enhancement Adaptions and new features discuss / RFC labels Feb 20, 2024
@jsmeix jsmeix added this to the ReaR v2.8 milestone Feb 20, 2024
@jsmeix jsmeix requested a review from a team February 20, 2024 13:41
@jsmeix jsmeix self-assigned this Feb 20, 2024
@jsmeix
Copy link
Member Author

jsmeix commented Feb 20, 2024

I tested it with an adapted ErrorIfDeprecated()

function ErrorIfDeprecated () {
    { (( $# >= 2 )) || BugError "Must call ErrorIfDeprecated with at least 2 arguments - feature and reason"
      local feature="$1" ; shift
      local reason="$*"

      if IsInArray "$feature" "${DISABLE_DEPRECATION_ERRORS[@]}" ; then
          LogPrint "Disabled deprecation error for '$feature'"
          return 0
      fi

      local text="Reason:
                  $reason

                  This feature is phased out in ReaR and will be eventually removed.
                  If it is indispensable, go to https://github.com/rear/rear/issues
                  and create an issue that explains why there is no alternative to it.

                  To disable this error and continue using this feature for now, set
                  DISABLE_DEPRECATION_ERRORS+=( $feature )
                 "
    } 2>>/dev/$DISPENSABLE_OUTPUT_DEV
    Error "Deprecation of '$feature'$LF$( TextPrefix '' <<<"$text" )"
}

which results for me this output

# usr/sbin/rear mkrescue
ERROR: Deprecation of 'gpt_sync_mbr'
Reason:
The 'gpt_sync_mbr' partitioning is no longer supported by SUSE since 2016
see https://github.com/rear/rear/issues/3148

This feature is phased out in ReaR and will be eventually removed.
If it is indispensable, go to https://github.com/rear/rear/issues
and create an issue that explains why there is no alternative to it.

To disable this error and continue using this feature for now, set
DISABLE_DEPRECATION_ERRORS+=( gpt_sync_mbr )

@jsmeix
Copy link
Member Author

jsmeix commented Feb 20, 2024

As another test I replaced in lib/_input-output-functions.sh
some

... | sed -e 's/^/  /' | ...

with

... | TextPrefix | ...

(plain 'TextPrefix' adds two spaces indentation).

@jsmeix
Copy link
Member Author

jsmeix commented Feb 20, 2024

@rear/contributors
please have a look here and provide feedback
what you think about such a TextPrefix function.

@jsmeix
Copy link
Member Author

jsmeix commented Feb 22, 2024

@rear/contributors
as no news is good news
I will merge it tomorrow afternoon
unless there are severe objections.
This is only an internal helper function
so we can change it as we like if needed.

@jsmeix jsmeix merged commit b873731 into master Feb 23, 2024
20 of 21 checks passed
@jsmeix jsmeix deleted the jsmeix-TextPrefix branch February 23, 2024 14:14
lzaoral pushed a commit to lzaoral/rear that referenced this pull request Feb 27, 2024
New TextPrefix function to
remove leading space and tab characters from input lines
and prefix each line with the argument string if specified.
The intent is to be able to properly indent multi-line message strings in the code
and output the message without the code indentation but prefixed as needed,
see rear#3160
@jsmeix
Copy link
Member Author

jsmeix commented Feb 27, 2024

The new TextPrefix needs to be fixed
because currently:

# function TextPrefix () { local prefix="${1-  }" ; sed -e "s/^[ \t]*/$prefix/" ; }

# (set -x ; echo foo | TextPrefix '/prefix/dir/' )
+ echo foo
+ TextPrefix /prefix/dir/
+ local prefix=/prefix/dir/
+ sed -e 's/^[ \t]*//prefix/dir//'
sed: -e expression #1, char 13: unknown option to `s'

The fix is to escape all / with / in prefix

# function TextPrefix () { local prefix="${1-  }" ; sed -e "s/^[ \t]*/${prefix//\//\\/}/" ; }

# (set -x ; echo foo | TextPrefix '/prefix/dir/' )
+ echo foo
+ TextPrefix /prefix/dir/
+ local prefix=/prefix/dir/
+ sed -e 's/^[ \t]*/\/prefix\/dir\//'
/prefix/dir/foo

The fix will be done "by the way"
in the related pull request
#3166
via
1c047d9

jsmeix added a commit that referenced this pull request Feb 27, 2024
Fix the TextPrefix function:
In the prefix value escape all / by \/
otherwise sed -e "/.../.../" gets invalid syntax
see #3160 (comment)
lzaoral pushed a commit to lzaoral/rear that referenced this pull request Mar 5, 2024
New TextPrefix function to
remove leading space and tab characters from input lines
and prefix each line with the argument string if specified.
The intent is to be able to properly indent multi-line message strings in the code
and output the message without the code indentation but prefixed as needed,
see rear#3160
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Adaptions and new features fixed / solved / done
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant