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

Updated configurable keys #788

Merged
merged 23 commits into from Aug 28, 2018

Conversation

Projects
None yet
5 participants
@Grimler91
Copy link
Member

Grimler91 commented Jul 29, 2018

Here's an rebased and updated version of @neverwin's configurable keys (#650). Ping @robertvandeneynde since you had good input in previous PR.

Some stuff remains: I haven't managed to get the property parser to accept arrow keys in termux.properties, if I for example try extra-keys = [['ESC','/','―','HOME','↑','DEL','-'],['TAB','CTRL','ALT','←','↓','→','PGDN']] I get:

ss_2

Which I suppose is a UTF-8/UTF-16 error, and should be fixable..
Writing extra-keys = [['ESC','/','\u2015','HOME','\u2191','DEL','-'],['TAB','CTRL','ALT','\u2190','\u2193','\u2192','PGDN']] instead works:

ss_1

I also think that the keys take up too much vertical space, making the rows smaller would be nice. @xqdoo00o I suppose that this is what you are doing in #759? But that works only for android 5? I didn't notice any big difference when I merged it (https://github.com/Grimler91/termux-app/tree/configurable_keys_smaller)

Also, more importantly, we/I need to add more validation of the extrakeys field! If, for example, two rows of different lengths are given the app crashes on start-up (with no change of fail-safe session).


I've also set the default extra keys to default to the single row we had before, and made the extra keys visible per default. Users who want several rows can configure it.

We should definitely ship a default termux.properties file with all the different options as comments, they are a bit secret as mentioned in the previous PR.

I've also kept the "swipe up from - for |" and "swipe up from / for " features, though they are less needed with configurable keys.

neverwin and others added some commits Apr 6, 2018

ExtraKeys: fix so app doesn't crash if ctrl/alt aren't in extrakeys
Otherwise we get:
AndroidRuntime: java.lang.NullPointerException: Attempt to invoke virtual method 'boolean android.widget.CompoundButton.isChecked()' on a null object reference
AndroidRuntime:        at com.termux.app.ExtraKeysView.b(SourceFile:128)
@Grimler91

This comment has been minimized.

Copy link
Member

Grimler91 commented Jul 29, 2018

Fixes #749.

@robertvandeneynde

This comment has been minimized.

Copy link
Contributor

robertvandeneynde commented Jul 30, 2018

Very nice ! Thanks ! I was just about to start coding the PR myself. But I prefer to think about the functionalities and summarize them as I did. Giving good input or good specs and let experts implement the feature. 😄

@robertvandeneynde

This comment has been minimized.

Copy link
Contributor

robertvandeneynde commented Jul 30, 2018

Also, more importantly, we/I need to add more validation of the extrakeys field! If, for example, two rows of different lengths are given the app crashes on start-up (with no change of fail-safe session).

That answers my question I had in the other PR. So yes, validation or default behavior, choose wisely. As goes the saying having good defaults is important, so is configuration.

For the case two lines of different length I'd say no error, it means all width = 1 (see my definition of width in the other PR), it's alright not to have a grid, our real hardware keyboards have hex layouts. 😄

I've also set the default extra keys to default to the single row we had before, and made the extra keys visible per default. Users who want several rows can configure it.

Well, choose wisely the good defaults ! I think the people that don't know about configuration (beginners?) prefer the ↑↓ arrows as mandatory for a shell and don't care ←→ are very nice too but less important. Beginners don't know about the configuration until we don't write a good settings menu. The dash "-" and "/" is convenient but easily found on all the keyboards. The "|" is less used by beginners (and again found on all the keyboards). We should focus the default to be on missing keys of classic android keyboard, but that are still widely use. ←→↓↑ are cleary it. And the 4, the Alt is useful for people knowing that emacs basic shortcuts like Alt+F exist.

For the one line order, I think "←→↓↑" is a good choice, because the most useful key is at the right. "←↓↑→" is my second advice because it's VIM like.

JSON

It's JSON isn't it ? What kind of format termux.properties is made of ? I see you use single quote but they are not allowed in JSON, so is it a typo?

We should definitely ship a default termux.properties file with all the different options as comments, they are a bit secret as mentioned in the previous PR.

👍 And there is the wiki.

I've also kept the "swipe up from - for |" and "swipe up from / for " features, though they are less needed with configurable keys.

I've thought about this feature, it's only useful if it's explicitly written in the termux.properties. Nobody would think of slide-up if they're not told to. People try to press, sometimes to Long press, never to slide up, that's weird.

However, if someone reads you can write "slideup": {"key": "|"} and tadaa, there it is ! it can become very handy to them.

Which I suppose is a UTF-8/UTF-16 error, and should be fixable

Again, what's the specification format for termux.properties ? If there is none, I suggest :

  • Be UTF-8, because it's not the year 2000 anymore.
  • If the file begins with emacs style #coding: cp1252, then it declares the encoding. In python, we use a the simple regex ^[ \t\f]*#.*?coding[:=][ \t]*([-_.a-zA-Z0-9]+) as defined in the PEP.

Maybe your encoding is not correctly set on your machine, change all your tools to utf-8 I'd say. 😄

I also think that the keys take up too much vertical space

They do, on your screen, on mine they are less tall than that #HopeThisHelps

case "":
chars = "-";
break;
case "-":
chars = "-";
break;

This comment has been minimized.

@robertvandeneynde

robertvandeneynde Jul 30, 2018

Contributor

Could be refactored:

case "":
case "-":
    // if it's "−" or "-", it's the same output
    chars = "-";
    break;

Including the comment, not so much people know the syntax.

@@ -71,9 +75,15 @@ static void sendKey(View view, String keyName) {
case "":
keyCode = KeyEvent.KEYCODE_DPAD_DOWN;
break;
case "":

This comment has been minimized.

@robertvandeneynde

robertvandeneynde Jul 30, 2018

Contributor

Add case "ENTER": too, of course the displayed chars must be "↲" but if people write ENTER in termux.properties it should work as expected.

@@ -59,6 +57,12 @@ static void sendKey(View view, String keyName) {
case "PGDN":

This comment has been minimized.

@robertvandeneynde

robertvandeneynde Jul 30, 2018

Contributor

Add aliases, PAGE_DOWN, PAGEDOWN etc. People are likely to have that in their termux.properties.
Same for INSERT, DELETE, BACKSPACE, UP DOWN LEFT RIGHT etc. A short name that is the preffered one, and longer name explicit.

@@ -251,7 +276,7 @@ public void run() {
scheduledExecutor = null;
}
if (longPressCount == 0) {
if (popupWindow != null && "―/".contains(buttonText)) {
if (popupWindow != null && "―/-".contains(buttonText)) {

This comment has been minimized.

@robertvandeneynde

robertvandeneynde Jul 30, 2018

Contributor

In the case we have buttonText.length != 1, I would go for:

Arrays.asList("", "/", "-").contains(yourValue)
case "":
chars = "-";
break;
case "-":
chars = "-";
break;

This comment has been minimized.

@robertvandeneynde

robertvandeneynde Jul 30, 2018

Contributor

What if the guy wrote "\u2015" in the termux properties? We should remove that case, if he wrote "\u2015" it means he wants to write U+2015 ― HORIZONTAL BAR. Not U+002D - HYPHEN-MINUS.

The default config should contains the ascii dash (U+002D - HYPHEN-MINUS), but should be displayed as "−" (U+2212 − MINUS SIGN) in the shell. See my display parameter in the PR.

@robertvandeneynde

This comment has been minimized.

Copy link
Contributor

robertvandeneynde commented Jul 30, 2018

After reading the source code termux.properties is simply a file that must be in $HOME/.termux directory. Or in $HOME/.config/termux/ of type java.util.Properties. Which makes sense.

However, also considering JSON has some advantages.

This .properties yields an error:

extrakeys = [
    ["←", "→", "↓", "↑"],
    ["CTRL", "ALT", "DEL", "TAB"]
]

Because new lines must be escaped:

extrakeys = [\
    ["←", "→", "↓", "↑"],\
    ["CTRL", "ALT", "DEL", "TAB"]\
]

Which is annoying for that case.

The same information is available in JSON:

{"extrakeys": [
    ["←", "→", "↓", "↑"],
    ["CTRL", "ALT", "DEL", "TAB"]
]}

I suggest that the code looks up for termux_config.json in both directories, and termux.properties in both directories. Merge the keys but yields a Warning if both files have keys in common, in that case, take the one in .properties.

The only advantage is that one doesn't have to put quotes around all the keys and values for simple cases like we generally have. Another advantage is that Comments are allowed. And that new lines don't have to be written "\n". However everybody does know Json, less people know java.util.properties but the syntax is so intuitive that's alright. Both syntaxes allow \u1234 syntax, neither does allow \U12345678 syntax to input emojis by unicode.

One option would be to be more lenient on the json file:

  • Removes comments with '#', ie. lines BEGINNING with '#'. As in .properties, it is not allowed to have some text THEN a comment, a comment must be on one line.
  • If the file is not parsable as JSON object, add "{}" around it, if it isn't still possible, throw an error.

We could switch to YAML, but less people know about it. JSON is simple but more verbose.

@Grimler91

This comment has been minimized.

Copy link
Member

Grimler91 commented Jul 30, 2018

Giving good input or good specs and let experts implement the feature.

I'm definitely no java expert though, but hey, thanks.

So about the termux.properties file. As you've already said it's just a normal utf-8 file, which is parsed by java.util.Properties into a properties object. The extrakeys entry is read into a string which is later parsed by the JSONArray library into a json object. So the string must have a syntax that JSONArray can handle.

' instead of " works just fine.

The reason I mentioned utf-16 is because java seem to use utf16 for strings, so maybe java.util.properties reads the utf-8 file as utf-16 and causes the problems. I don't really know, have to do more testing.

Anyways, good that the development is on it's way!

@Grimler91

This comment has been minimized.

Copy link
Member

Grimler91 commented Jul 30, 2018

After reading the source code termux.properties is simply a file that must be in $HOME dir. Or in $HOME/.config/termux

Actually $HOME/.termux/or $HOME/.config/termux. I managed to find the wiki entry on it: https://wiki.termux.com/index.php?title=Terminal_Settings

@robertvandeneynde

This comment has been minimized.

Copy link
Contributor

robertvandeneynde commented Jul 31, 2018

I might fork the repo and add commits to the pull request, if I find how to do do that, do I have to do another PR? Or can I join in? I know git and branches but pretty new to github.

I'm definitely no java expert though, but hey, thanks.

Cmon, a little bragging doesn't hurt, and expert doesn't have a good definition (this word makes me think about this humorous video).

I managed to find the wiki entry for the configuration file location

And in the source code.

UTF-16

Indeed, java does use UTF-16, sizeof(char) is 2 bytes. But in your screenshot, you seem to have 3 characters, which is utf-8 for the arrow keys. So I'd say it's utf-8 but should be utf-16. Thaaat's why I suggest the first line to be # coding: utf-8 to specify the encoding and

And the default keys should be ascii. The user can write strings like "LEFT", "RIGHT", "UP", "DOWN" for Arrow keys and control characters. That would be Displayed as ←↑→↓ (↲ Enter) (↹ Tab). And if the user doesn't want that he can use the "display" property I was thinking of:

extrakeys = [[{"key":"TAB", "display":"TAB"}, {"key":"CTRL", "display":"^"}]]

Or we can add a new option:

extrakeys = [["TAB", "CTRL"]]
extrakeys.controlcharstext = ["TAB", "ENTER"]

This would mean, these keys shall be rendered as "TAB" and "ENTER" and not their default (↲ Enter, ↹ Tab) but I prefer the "display" per key.

@Grimler91

This comment has been minimized.

Copy link
Member

Grimler91 commented Jul 31, 2018

I might fork the repo and add commits to the pull request, if I find how to do do that, do I have to do another PR? Or can I join in? I know git and branches but pretty new to github.

Sounds great! I probably won't work on this again for another day or two.
If you open a PR to my branch (i.e. go to https://github.com/grimler91/termux-app/tree/configurable_keys and press "New pull request" there) then your commits are added to this pull request.

Indeed, java does use UTF-16, sizeof(char) is 2 bytes. Butin your screenshot, tou seem to have 3 characters, which is utf-8 for the arrow keys.

Yupp, I checked, it's 3 characters. So we need to decode and encode it as utf-16 instead in ExtraKeys.java I guess. I don't see how # coding: utf-8 would help (and I tried adding it to termux.properties, no difference).

You have lots of good ideas that could probably be split into several PRs, which this first one aiming at making it possible to configure keys (without breaking termux easily) and then enhance it by increasing user-friendliness and adding more options and possibilities.

@robertvandeneynde

This comment has been minimized.

Copy link
Contributor

robertvandeneynde commented Jul 31, 2018

Okep, I'll pull request You then.

Indeed java.util.Properties sees # coding: utf-8 as a comment, but that would be something clever we would implement so that the user can specify the encoding he used. But because it's stupid to reinvent the wheel we'll take the convention of emacs, the same as the one from Python, we'd use that regex:

^[ \t\f]*#.*?coding[:=][ \t]*([-_.a-zA-Z0-9]+)

You have lots of good ideas that could probably be split into several PRs

Yep of course. The PR serves also as a conversation, We Could talk about the ideas in a Issue though, then go for multiple PR tagging that issue.

However, the first version of the implementation should be thought, not just this sounds good to me, here's my PR and fornwall would be like This is great, I'll push it on the store aaand the new user will complain Why do you put two bars and no configurable keys.

Okep, back to coding, however I currently can't test the app myself (pretty new to android native app), how do I do that? 😄 At first I'll do the Java-Only parts.

@robertvandeneynde

This comment has been minimized.

Copy link
Contributor

robertvandeneynde commented Aug 1, 2018

For the encoding issue, in TermuxPreferences.java:

try (FileInputStream in = new FileInputStream(propsFile)) {
    props.load(new InputStreamReader(in, "utf-8"));
}

Instead of props.load(in);.

The java.util.Properties class has two load functions seen here. The old code used a byte oriented class and we want a character oriented one, the old-code assumed latin1 encoding which is not a good default anymore.

@robertvandeneynde

This comment has been minimized.

Copy link
Contributor

robertvandeneynde commented Aug 1, 2018

Alright, I just added two commits, but I didn't test it because I haven't installed the android sdk yet. If you could compile and test for me that'd be good.

  • Default file uses "LEFT" instead of "←", that's more easy for people to input and doesn't have encoding problems.
  • Refactoring of code using Ctrl, Alt, Fn. Many copy paste, I refactored that.
@Grimler91

This comment has been minimized.

Copy link
Member

Grimler91 commented Aug 1, 2018

I'll test your changes and give you a report :)

About building the app, you can install the sdk (and ndk) by running scripts/setup-android-sdk.sh in termux-packages. You also need to have java installed (obviously).

After that, you can build the app pretty easily from the command line (without the need of installing android-studio). I have cloned all the termux apps to a common folder and from that folder I run this script:

#!/bin/sh                                                                                                                                                                                                                                                                                                                   

### passwords for keystore                                                                                                                                                                                                                                                                                                  
# keystore_pw=$(pass dev/keystore)                                                                                                                                                                                                                                                                                          
# key_pw=$(pass dev/keystore-termux)                                                                                                                                                                                                                                                                                        

### your keystore file                                                                                                                                                                                                                                                                                                      
# keystore_file=NAME.keystore                                                                                                                                                                                                                                                                                               

export ANDROID_HOME=~/lib/android-sdk
export ANDROID_NDK_HOME=~/lib/android-ndk

if [ -z $1 ]; then
    apps="app api float tasker styling boot"
else
    apps="$1"
fi

mkdir -p apps

for app in $apps; do
    if [ ! -d "termux-$app" ]; then
        git clone git@github.com:termux/termux-$app
    fi

    cd termux-$app
    ### Name the apk after the branch
    branch=$(git rev-parse --abbrev-ref HEAD) 

    ./gradlew clean
    ### Build with gradle:                                                                                                                                                                                                                                                                                                    
    ./gradlew assemblerelease
    ### Align the app (not strictly necessary)                                                                                                                                                                                                                                                                                
    zipalign -v -p 4 app/build/outputs/apk/release/app-release-{unsigned,aligned}.apk
    ### Sign the app:                                                                                                                                                                                                                                                                                                      
    # jarsigner -sigalg SHA1withRSA -digestalg SHA1 \                                                                                                                          
    #         -keystore ~/.android/${keystore_file} \                                                                                                                              
    #         -storepass ${keystore_pw} \                                                                                                               
    #         -keypass ${key_pw} \                                                                                                                                               
    #         app/build/outputs/apk/release/app-release-aligned.apk termux_app
    #### Move the finished apk to folder "apps"
    cp app/build/outputs/apk/release/app-release-aligned.apk ../apps/termux-$app-$branch.apk
    cd ..
done

If you want to sign the app as well you need to generate a signing key and then un-comment those things in the script.

Remember that you can't have several termux apps installed if they are signed with different keys, i.e. you need to uninstall termux-{app,api,...} fully before installing your own version, so remember to backup $HOME if you do this.

Grimler91 and others added some commits Aug 1, 2018

Merge pull request #1 from robertvandeneynde/configurable_keys
Basic configuration of extrakeys and arrows for a lot of characters
Merge pull request #2 from robertvandeneynde/configurable_keys
Fix refactoring, Ctrl, Alt, Fn keys work again
@robertvandeneynde

This comment has been minimized.

Copy link
Contributor

robertvandeneynde commented Aug 1, 2018

About JSON version, the org.json package is included by default in android, I'm wondering if it uses the same api as this project on Maven at its latest version? Here is the link on github.

From the doc, single quotes are normal, and often don't need to even used !

The texts produced bfy the toString methods strictly conform to the JSON syntax rules. The constructors are more forgiving in the texts they will accept:

  • An extra , (comma) may appear just before the closing brace.
  • Strings may be quoted with ' (single quote).
  • Strings do not need to be quoted at all if they do not begin with a quote or single quote, and if they do not contain leading or trailing spaces, and if they do not contain any of these characters: { } [ ] / \ : , # and if they do not look like numbers and if they are not the reserved words true, false, or null.

So the default can be written:

extra-keys = [[ESC, CTRL, ALT, TAB, "-", "/", "|"]]

The last three don't have to be escaped, but I prefer it like this.
Backslashs are annoying, i might want to add a BACKSLASH alias :p

That's the correct spelling:

extra-keys = [[ENTER, "\\\\", ALT]]

They also have a org.json.Property to work along with java.util.Properties 🤔

@Grimler91

This comment has been minimized.

Copy link
Member

Grimler91 commented Aug 2, 2018

I've invited you to collaborate (you should have an email from github) to my grimler91/termux-app fork.

I merged in changes from master and then cherry-picked your commit. With cherry-pick I mean git fetch robert configurable_keys; git cherry-pick {commit-hash}. It gives a bit cleaner history if the branch histories aeen't identical (no merge needed).

So the default can be written:

extra-keys = [[ESC, CTRL, ALT, TAB, "-", "/", "|"]]

I think that look more confusing, better to quote all strings then, even if it isn't strictly needed.

Wait, "\\" is for one backslash? Wow... An alias would indeed be nice then!

@robertvandeneynde

This comment has been minimized.

Copy link
Contributor

robertvandeneynde commented Aug 2, 2018

Yes, 4 backslashes for a single backslash, because Both Json and Properties use \\ for a backslash.
That's why BACKSLASH is the most useful ascii alias, QUOTE and APOSTROPHE, seems good too. And if people what to be sure about escapes, we should consider ~/.termux/termux_properties.json(or is the . a better idea than _?)

Adding other ascii aliases seems a bit stupid (but we had to, I'd pick all unicode names and aliases or all ascii names in unix /usr/share/X11/xkb/symbols aka xkb_symbols because lots of people know them), but I prefer people write "-", "/" better than...

HYPHEN-MINUS (uniname), SOLIDUS (uniname), REVERSE SOLIDUS (uniname), REVERSE-SOLIDUS (uniname.replace(' ', '-')), BACKSLASH (X11 name), TILDE (uniname), ASCIITILDE (X11 name), GRAVE ACCENT (uniname), GRAVE (X11 name), BACKTICK (slang), QUOTATION MARK (uniname), APOSTROPHE (uniname).

However, aliases for controls characters is an idea, ^D \u0004 <EOT> END OF TRANSMISSION for example, or \u0007 <BEL> BELL. We already have U+0008 \x08 <BS> BACKSPACE after all. I'm, I named BKSP but the small unicode alias is BS. 🤔

put("PAGE-DOWN", "PGDN");

put("DELETE", "DEL");
put("BACKSPACE", "BKSP");

This comment has been minimized.

@robertvandeneynde

robertvandeneynde Aug 2, 2018

Contributor
put("BACKSLASH", "\\");
put("QUOTE", "\"");
put("APOSTROPHE", "'");
@robertvandeneynde

This comment has been minimized.

Copy link
Contributor

robertvandeneynde commented Aug 2, 2018

@Grimler91 In your comment below the screenshot, write that you wrote extra-keys = [["ESC", "/", "ENTER", "HOME", "UP", "DELETE", "-"], ["TAB", "CTRL", "ALT", "LEFT", "DOWN"]]. People might want to read the PR to see the changes, this gives a good idea of the implemented features when just reading the PR. Also remove your first screenshot where you had an encoding problem.

@robertvandeneynde

This comment has been minimized.

Copy link
Contributor

robertvandeneynde commented Aug 2, 2018

@fornwall this PR is ready to be merged in!

A screenshot explains the thing when extra-keys = [["ESC", "/", "ENTER", "HOME", "UP", "DELETE", "-"], ["TAB", "CTRL", "ALT", "LEFT", "DOWN"]] in set in termux.properties.

Release notes include:

  • Extra keys are enabled by default and include UP and DOWN arrow keys by default on one single line.
  • Extra keys are configurable by adding extra-keys attribute in ~/.termux/termux.properties to customize layout of extra keys. The old one line bar can be used with extra-keys = [["ESC", "CTRL", "ALT", "TAB", "-", "/", "|"]] or the new two lines bar can be used with extra-keys = [["ESC", "/", "-", "HOME", "UP", "END", "PGUP"], ["TAB", "CTRL", "ALT", "LEFT", "DOWN", "RIGHT"]]. All the possible keys can be seen in the source code in the map keyCodesForString in ExtraKeysView.java, aliases like RETURN for ENTER can be seen in the map controlCharsAliases.
@tomty89

This comment has been minimized.

Copy link
Contributor

tomty89 commented Aug 2, 2018

Is it possible to use /data/data/com.termux/shared_prefs/com.termux_preferences.xml for this instead?

@Grimler91

This comment has been minimized.

Copy link
Member

Grimler91 commented Aug 2, 2018

@tomty89 what's the benefit of that? Feels like it would make the file much harder to find.

@Grimler91 Grimler91 changed the title Updated configurable keys [WIP] Updated configurable keys Aug 2, 2018

@tomty89

This comment has been minimized.

Copy link
Contributor

tomty89 commented Aug 2, 2018

It's not really much harder, but yeah it might feel so (and out-of-comfort-zone).

Well there are couple reasons (which might be a bit personal and may or may not be valid):

  1. Not a lover of dotfiles, especially when it's not even related to the shell or the "terminal", but rather the "app"
  2. The show extra key attribute makes use of that. Feels more consistent if they are in the same place. (Though you may argue that that can be moved instead...)
  3. Perhaps xml and/or the SharedPreference interface would makes it even more neat? Only a hunch though.
@robertvandeneynde

This comment has been minimized.

Copy link
Contributor

robertvandeneynde commented Aug 2, 2018

The app currently uses SharedPreference interface for some settings (I've seen it somewhere in the source code), I don't know for what. But it is but it seems to be a classic way to store settings in android. We could add that in another small PR, the idea would be also to have a classic "settings" menu when some useful settings will be changed in a gui, the dotfile would override. We currently look for multiple locations and that's a good thing.

About xml, we could also add a simple xml interface when we'll add JSON (currently only java.util.Properties type is allowed), if people likes xml more than JSON or properties, why not. However, xml is verbose, JSON less, properties is a good tradeoff.

On Ubuntu, that's standard to keep app infos in ~/.config, because the $HOME dir is related to the user whereas /etc is about root. When people back their stuff, they backup $HOME.

About the directory /data/data/com.termux/, if it's standard on Android app (I'm no Android expert), we'll add the location.

About the filename, com.termux_preferences, is that standard? Or just your first idea?

@tomty89

This comment has been minimized.

Copy link
Contributor

tomty89 commented Aug 2, 2018

The file is created by the SharedPreference interface. I'm sure you have it in your Termux as well. That's a reason I think it is preferred, as it seems to be the orthodox one where you don't even need to deal with the path yourself.

What I consider bad is that Termux has been making use of two different interfaces (android.content.SharedPreferences and java.util.Properties) in TermuxPreferences for different stuff. I am not sure there's any good reason doing so. That's why I think we should stop using one of them and ditch that completely one day.

Semantically speaking I don't think we should put app settings (i.e. settings that has nothing to do with programs we uses in Termux or terminal emulation) anywhere under /data/data/com.termux/files but somewhere else (even directly) under /data/data/com.termux. And certainly we don't need to follow XDG specs (where ~/.config comes from) for that.

@robertvandeneynde

This comment has been minimized.

Copy link
Contributor

robertvandeneynde commented Aug 2, 2018

Ok I see, the file in shared_prefs currently only contains show_extra_keys, current_session and fontsize.

The current advantage of ~/.config is that users can cd ~/.config in the terminal, whereas cd /data/data/com.termux/shared_prefs is longer. But we could add a symlink ln -s /data/data/com.termux/shared_prefs/com.termux_preferences.xml ~/.termux_preferences.xml or similar.

And again, the advantage of java.util.Properties is that it's very brief and not verbose. But I agree this does the trick too:

<?xml version='1.0' ?>
<map>
    <string name="extra-keys">[
        ["CTRL", "ALT", "DELETE"]
    ]</string>
</map>

Xml does ask more questions, <string name="extra-keys"> vs <json name="extra-keys">? Don't use json and use xml?

<!-- very verbose -->
<?xml version='1.0' ?>
<map>
    <rows name="extra-keys">
        <row>
            <key>CTRL</key>
            <key>ALT</key>
            <key>DELETE</key>
        </row>
    </rows>
</map>

I prefer the <string name="extra-keys">[["CTRL", "ALT", "DELETE"]]</string> from far, the xml does ask too much questions (should I use CTRL vs etc.).

What is the point of view of @fornwall ? Why did you choose ~/.config and java.util.Properties instead of SharedPreferences for the keys?

However I think this can be in a future PR, and using something standard for android is nice. As the dev guide says, SharePreferences is useful for storing simple key/value data, which is what our .properties does.

@robertvandeneynde

This comment has been minimized.

Copy link
Contributor

robertvandeneynde commented Aug 8, 2018

However, I think the new default must be reconsidered:

  • people like one line
  • people like arrow keys, especially UP and DOWN (it's a terminal!)
  • the default width seems to be 7
  • the - / | are useful but already there on all keyboards.

I suggest the new default being:

extra-keys = [["ESC", "TAB", "CTRL", "ALT", "-", "DOWN", "UP"]]

I don't think there is enough space to have both "- / |" and "↓↑".

The choice of "-" is kind of arbitrary, but I think "-" is more useful, to provide options and termux-like-commands.

Also, the order ESC TAB CTRL ALT seems better too, because ESC is expected to be to the left-most (useful in vim), TAB also, and about CTRL and ALT, they are always followed by another letter.

And of course, we'll find later a way to explain to people how to write the .properties file on the wiki and/or doing a proper Preferences panel easily accessible.

@robertvandeneynde robertvandeneynde force-pushed the Grimler91:configurable_keys branch from a239157 to f45c57c Aug 8, 2018

@robertvandeneynde

This comment has been minimized.

Copy link
Contributor

robertvandeneynde commented Aug 15, 2018

@fornwall any feedback/approbation?

@fornwall fornwall merged commit 3693e3c into termux:master Aug 28, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@fornwall

This comment has been minimized.

Copy link
Member

fornwall commented Aug 28, 2018

@robertvandeneynde Thanks a lot, and sorry for the delay!

I'll try it out during the coming couple of days and make a release shortly.

@robertvandeneynde

This comment has been minimized.

Copy link
Contributor

robertvandeneynde commented Aug 28, 2018

I figured out you were on vacation :)

I'm still wondering if the default should be on one line with arrow keys like in my last commit or on two lines, and if show_extra_keys should be displayed by default (I think yes, majority of people don't know about changing keyboards or can't plug a real keyboard).

And also, because the left menu cannot be found easily, add an entry "show side menu" to the long press menu. And a text "long press on the screen to see the options" in the welcoming message in the terminal.

For the future, I think some accessibility are useful, I'm seeing a classic "settings" (Preferences) panel. Such that basic settings can be changed in a classic way, without editing dot files.

@robertvandeneynde

This comment has been minimized.

Copy link
Contributor

robertvandeneynde commented Oct 16, 2018

@fornwall ? It's been 2 months when you said "soon" :p Isn't there someone else that can test and publish a new version on the store ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment