Arch Linux User Repository

@ant0n et all, lets keep the comments here about packaging issues, general Brave usage issues should go in another forum to not clutter up this comment space. I'm deleting comments that have no relation to packaging. Grey areas like crashes that could be blamed on Arch can stay until proven otherwise, but things like how to configure Brave to handle popups or site X or whatever just don't belong here. Thanks for understanding.

I agree with wknapik. You are trying to fix a non-existent issue and really don't need to do that. It's enough for anybody to pay enough attention what they are adding to brave-flags. It's impossible to specify every single mistake idiots are capable of. There are legion of them and they are very inventive.

For now, I will be reverting the changes. I get the point that these can help in some situations. But right now, since nobody complained about not being able to use special chars like * on their flags, is not fixing anything and only giving some problems to other users.

I'm think I can (But have to check) if a message can be printed when updating the package warning users of the changes and what needs to be done.

Also I'm curious about @heapifyman and the Manjaro Sway. Is the brave-bin package there this one + preconfigured config file for Wayland? Is it written on some wiki? If I understood correctly there the config file is provided by the distro flavor devs, so I might be braking things for users without them knowing, looks like the patches are good, but maybe I need a way to communicate breaking changes specially regarding people where the config file is preconfigured.

I don't like the fact that we are breaking previously running configs for others.

In chromium/chrome, this feature is a part of the browser. Here, it's implemented in 7 lines of code, so there will be limitations.

My regex skills are not at the point where I can understand that last sed, to be honest.

There you go:
- s/\s*#.*//g - remove comments and whitespace before them
- s/^\s*//g - remove whitespace at the beginning of each line
- s/\s*$//g - remove whitespace at the end of each line
- /\S/p (with -n) - print only lines that contain at least one non-whitespace character (equivalent to grep '\S')

are there chrome flags that accept * or white spaces to begin with or are we fixing imaginary issues?

There are many special characters in bash https://mywiki.wooledge.org/BashGuide/SpecialCharacters

Allowing the shell to expand them in this case is a bad idea IMO.

Up to you @alosarjos. Either revert my patch and accept the shell expansion, or apply my second patch and accept the limitations in format (each option on a new line and no quotes). There's also a third option of implementing this properly, which would take more code than those 7 lines.

Hi everyone, sorry for the late response.

I usually test all this on my PC to check everything keeps working, but turns out that, since I have a bunch of flags I already have them all in separate lines, so I didn't see any breaking changes.

I don't like the fact that we are breaking previously running configs for others. My regex skills are not at the point where I can understand that last sed, to be honest.

I don't see harm on the new changes though, but are there chrome flags that accept * or white spaces to begin with or are we fixing imaginary issues?

AFAIK, spaces must be escaped regarding the docs:

https://www.chromium.org/developers/how-tos/run-chromium-with-flags/

So I'm not sure at all if these patches are actually necessary, @wknapik, can you tell me what was failing to you before?

@alosarjos please apply also this patch to improve handling of comments and whitespace:

diff --git a/brave-bin.sh b/brave-bin.sh
index 0e657ed..30940d1 100644
--- a/brave-bin.sh
+++ b/brave-bin.sh
@@ -4,7 +4,7 @@ XDG_CONFIG_HOME="${XDG_CONFIG_HOME:-$HOME/.config}"
 # Allow users to override command-line options
 USER_FLAGS_FILE="$XDG_CONFIG_HOME/brave-flags.conf"
 if [[ -f "$USER_FLAGS_FILE" ]]; then
-   mapfile -t USER_FLAGS < <(sed 's/#.*//g' "$USER_FLAGS_FILE")
+   mapfile -t USER_FLAGS < <(sed -e 's/\s*#.*//g' -e 's/^\s*//g' -e 's/\s*$//g' "$USER_FLAGS_FILE"|sed -n '/\S/p')
 fi

 export CHROME_VERSION_EXTRA="stable"

This takes care of trimming whitespace around options and dropping lines that contain no options, which is relevant when you quote everything you pass on the command line.

PS. The approach you took to removing comments (which I haven't changed) would be problematic if a browser option was used with an argument containing the # character, like --some-option=foo#bar, but that seems like an unlikely edge case, so I think it's fine to favor simplicity of implementation here.

@heapifyman each option should be on a separate line (and should not be quoted, as the script takes care of that), instead of having multiple options on one line.

So this is bad

--enable-features=UseOzonePlatform --ozone-platform=wayland
--foo="bar baz"

This is good

--enable-features=UseOzonePlatform
--ozone-platform=wayland
--foo=bar baz

Some browser options may accept arguments containing spaces, so breaking up arguments on spaces is tricky. Doable, but the code would have to be aware of the context the space occurs in, so would be considerably more complex.

The latest version 1:1.39.111-2 with the patch by @wknampik does not seem to work with the default brave-flags.conf that is installed by Manjaro Sway.

brave-flags.conf has the following content, a single line:

--enable-features=UseOzonePlatform --ozone-platform=wayland

That should make brave run on wayland natively.

With the latest version of brave-bin.sh these settings seem to be ignored. Starting brave and then running xlsclient -a returns brave, meaning it runs on Xwayland, not native wayland, as far as I understand. Running xeyes confirms that - eyes are following mouse cursor when brave is focused.

If I revert the changes to the script then xlsclient -a does not return brave, anymore.

Question: is that an issue with the script or do I (and the Manjaro Sway developers) need to adjust anything in brave-flags.conf?

@wknampik is right, @alosarjos. Shebangs exist exactly for this reason to specify the needed shell interpreter. Besides, Bash is a dependency of base. Everybody has Bash.

@alosarjos bash is a dependency of multiple packages in base-devel, so it's guaranteed to be available. The shebang at the top of brave-bin.sh selects bash as the interpreter, so we're all set. I use zsh myself and this works fine for me.

@wknapik I see... The problem there is that bash is not mandatory as default shell, people may be using ZSH, so i tried there the mapfile, and doesn't recognize it, so I can't put that on the AUR script...