Skip to content

[WIP] RFC 3: languages #28

Open
shahrukh330 wants to merge 6 commits into
mainfrom
rfc-languages
Open

[WIP] RFC 3: languages #28
shahrukh330 wants to merge 6 commits into
mainfrom
rfc-languages

Conversation

@shahrukh330
Copy link
Copy Markdown
Contributor

@shahrukh330 shahrukh330 commented May 1, 2021

@shahrukh330 shahrukh330 requested a review from kalbasit May 1, 2021 16:42
Comment thread rfcs/0003-languages.md Outdated
Comment thread rfcs/0003-languages.md Outdated
Comment thread rfcs/0003-languages.md Outdated
Comment thread rfcs/0003-languages.md Outdated
Comment thread rfcs/0003-languages.md Outdated
Comment thread rfcs/0003-languages.md Outdated
Comment thread rfcs/0003-languages.md Outdated
`Array` of `str`, with apply function which iterate over the array and find
appropriate language or tool.

### Per-editor
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would probably not be editor specific. When you enable go, for example, you might want to have the Go compiler in your environment. It might also configure your shell prompt to change depending on whether you are in a Python virtual environment. It would probably be per-program or per-module.
Note: this is not the only place in the RFC where that nomenclature should be changed.

Comment thread rfcs/0003-languages.md Outdated
### Per-editor

Each editor that supports a language must provide a
`soxin.programs.<editor-name>.language` and `soxin.programs.<editor-name>.tool`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So shouldn't it be soxin.programs.<editor-name>.programming.language?

Comment thread rfcs/0003-languages.md Outdated
Comment on lines +140 to +141
# Examples and Interactions
[examples-and-interactions]: #examples-and-interactions
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is duplicated.

Comment thread rfcs/0003-languages.md
Comment on lines +144 to +145
# Drawbacks
[drawbacks]: #drawbacks
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One drawback I might add, is that we might not make the difference between soxin.programs and soxin.tools. What would it be and what are the criteria for putting a module under programs rather than tools?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a definition section at the top of the RFC to explain this?

My 2cents: go is a language but gorename is not, it's a tool. Go like any other language may also enable one or more tools.

Comment thread rfcs/0003-languages.md Outdated
Comment thread rfcs/0003-languages.md Outdated

Each program that supports a language must provide a
`soxin.programs.<program-name>.programming.language` and
`soxin.programs.<editor-name>.programming.tool` option to allow the user to
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should avoid specifying the type of the module. So instead of saying each program we should say each module.

Comment thread rfcs/0003-languages.md Outdated

let
cfg = config.soxin.programs.neovim;
goSupportRc = cfg.programming.languages.go ? "";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The returned type of this is an attrset, not a string; Shouldn't it be .go ? {}?

Comment thread rfcs/0003-languages.md Outdated
${gitSupportRc}
'';

plugins = [ /* Some plugins */ ] ++ goSupportPlugins ++ gitSupportPlugins;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about this, I don't think it should say go or git, instead, it should add the RC and plugins of all enabled language and tools in soxin's settings.

Comment thread rfcs/0003-languages.md
Comment on lines +144 to +145
# Drawbacks
[drawbacks]: #drawbacks
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a definition section at the top of the RFC to explain this?

My 2cents: go is a language but gorename is not, it's a tool. Go like any other language may also enable one or more tools.

* 'main' of github.com:SoxinOS/soxin: (21 commits)
  flake: follow nixpkgs on master (#44)
  treewide: move the project to using flake-utils-plus (#41)
  pkgs: deprecate stdenv.lib (#42)
  programs.starship: init module (#40)
  programs.less: init module (#40)
  programs.keybase: init module (#40)
  programs.git: init module (#40)
  programs.autorandr: init module (#40)
  modules: sort the list of modules per type (#40)
  lib: homeManagerConfiguration support for direct home-manager support (#37)
  themes.gruvbox-dark: add support for i3 and polybar (#36)
  programs.termite: initial import with theme support (#35)
  programs.tmux: initial import with theme support (#34)
  programs.zsh: initial import with theme support (#33)
  programs.neovim: initial import with theme support (#31)
  [lib] nixosSystem: allow setting global/nixos/hm modules and update to latest HM (#32)
  treewide: format all files
  programs.rofi: add support for swapping i3 workspaces (#30)
  programs.rofi: update to support master branch of HM (#30)
  programs.rofi: refactor: compute the modi in the apply function (#30)
  ...
Co-authored-by: Wael Nasreddine <wael.nasreddine@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants