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

Changing callback based on the grammar of the active text editor? #290

Open
aminya opened this issue Apr 7, 2020 · 8 comments
Open

Changing callback based on the grammar of the active text editor? #290

aminya opened this issue Apr 7, 2020 · 8 comments

Comments

@aminya
Copy link
Member

aminya commented Apr 7, 2020

Is it possible to change the callback for a button based on the grammar of the active text editor?

@aminya
Copy link
Member Author

aminya commented Apr 7, 2020

This issue seems related: #192

@suda
Copy link
Collaborator

suda commented Apr 8, 2020

Interesting idea. How would you propose to declare different callback s for different grammars?

@aminya
Copy link
Member Author

aminya commented Apr 8, 2020

  • One way is to just write a function that handles the changes and dispatches on separate callbacks for different grammar. This is independent of Tool-bar.
function mycallback() {
	// change callback based on the grammar
	let grammar = atom.workspace.getActiveEditor().getGrammar()
	if (grammar == "julia") {
	  // do this
	} else {
	  // do that
	}
}

The first way allows for more flexibility, which is currently possible anyways.

  • The second way is to provide another option in ButtonsOptions for different grammars, and make Tool-bar dispatch on the correct callback based on the active grammar:
  toolBar.addButton({
    icon: 'octoface',
    callback: {
        julia: "app:do-julia",
        python: "app:do-python"
      }
  });

The second way is nicer in my opinion. However, I don't know how we are going to handle its possible conflict with Key modifiers without breaking backward compatibility. Probably there is a way.

Maybe we can add an option to enable grammar dispatch:

  toolBar.addButton({
    icon: 'octoface',
	grammars: true, // enables grammars and breaks key modifiers
    callback: {
        julia: "app:do-julia",
        python: "app:do-python",
        c: {
          // now key modifiers should be under the individual grammar
          "": "app:do-c1",
          "shift": "app:do-c2"
        }
      }
  });

@ericcornelissen
Copy link
Contributor

ericcornelissen commented Apr 8, 2020

I like the idea, I don't think it would cause problems with key modifiers, personally, as I wouldn't use any other keys than shift, ctrl, etc. But of course, some people may use e.g. c (and there may be grammers that conflict with those keys as well...).

Perhaps we could make the grammar thing explicit from within the callbacks object, e.g.:

toolBar.addButton({
  icon: 'octoface',
  callback: {
    grammar: {
      julia: "app:do-julia",
      python: "app:do-python",

      // The "c" language
      c: "app:do-c-lang",
    },

    // the "c" key
    c: "app:do-c-key",
  }
});

Or distinguish grammers/languages from keys in the key name, e.g.:

toolBar.addButton({
  icon: 'octoface',
  callback: {
    // Using a "lang" or "language" prefix
    "lang-julia": "app:do-julia",
    "language-julia": "app:do-julia",

    // Using a "grammer" prefix
    "grammer-julia": "app:do-julia",

    // The you can use the "c" language
    "lang-c": "app:do-c-lang",
    // And the "c" key
    c: "app:do-c-key",
  }
});

Where only one or multiple prefixes can be supported.

@aminya
Copy link
Member Author

aminya commented Apr 8, 2020

My concern is about the case when someone wants to use both modifiers and grammars. We should allow this too. My example allowed this. See c language for example that has modifiers for itself (shift, when in C grammar).

  toolBar.addButton({
    icon: 'octoface',
	grammars: true, // enables grammars and breaks key modifiers.
					// now key modifiers should be under the individual grammar
    callback: {
        julia: "app:do-julia",
        python: "app:do-python",
        c: {
          "": "app:do-c1",
          "shift": "app:do-c2"
        }
      }
  });

@aminya
Copy link
Member Author

aminya commented Apr 8, 2020

Another way is to just avoid doing this, and add an option to addButton itself, and have separate buttons for different grammars. This doesn't change modifiers syntax too. It allows for different class or shape for different grammars, and it can also solve #192, since we can hide the buttons when they are not in their grammar.

  toolBar.addButton({
    icon: 'octoface',
	grammar: "julia",
    callback: "app:do-julia"
  });

  toolBar.addButton({
    icon: 'octoface',
	grammar: "python",
    callback: "app:do-python"
  });

@ericcornelissen
Copy link
Contributor

My concern is about the case when someone wants to use both modifiers and grammars. We should allow this too. My example allowed this. See c language for example that has modifiers for itself (shift, when in C grammar).

  toolBar.addButton({
    icon: 'octoface',
	grammars: true, // enables grammars and breaks key modifiers.
					// now key modifiers should be under the individual grammar
    callback: {
        julia: "app:do-julia",
        python: "app:do-python",
        c: {
          "": "app:do-c1",
          "shift": "app:do-c2"
        }
      }
  });

Of course! That would be possible with all proposals so far! If you want modifiers for the grammar callback you should specify it as an object.

Another way is to just avoid doing this, and add an option to addButton itself, and have separate buttons for different grammars. This doesn't change modifiers syntax too. It allows for different class or shape for different grammars, and it can also solve #192, since we can hide the buttons when they are not in their grammar.

  toolBar.addButton({
    icon: 'octoface',
	grammar: "julia",
    callback: "app:do-julia"
  });

  toolBar.addButton({
    icon: 'octoface',
	grammar: "python",
    callback: "app:do-python"
  });

This is quite a nice solution I think, definitely the most compatible with the current API. Unfortunately it would result in some duplication that is perhaps not desirable. 🤔

@aminya
Copy link
Member Author

aminya commented Apr 8, 2020

  toolBar.addButton({
    icon: 'octoface',
	grammar: "julia",
    callback: "app:do-julia"
  });

  toolBar.addButton({
    icon: 'octoface',
	grammar: "python",
    callback: "app:do-python"
  });

This is quite a nice solution I think, definitely the most compatible with the current API. Unfortunately it would result in some duplication that is perhaps not desirable. 🤔

Yes, I like this one more! Duplication could be solved by using variables.

let icon1 = 'octoface'

toolBar.addButton({
    icon: icon1
  	grammar: "julia",
    callback: "app:do-julia"
});

toolBar.addButton({
    icon: icon1
  	grammar: "python",
    callback: "app:do-python"
});

or more crazy:

let myoption = {
    icon: "octoface",
  	grammar: "julia",
    callback: "app:do-julia"
}

toolBar.addButton(myoption);

myoption.grammar = "python"
myoption.callback= "app:do-python"
toolBar.addButton(myoption);

If it was me I will just use separate variables to avoid duplication. 😄

@aminya aminya added this to TODO in Extend API Jun 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

3 participants