Skip to content

Conversation

@MiguelRoldao
Copy link

Added quoting modes, to enable different quotePairs for different files/filetypes based on user settings. Added more info the the help file. The 'repo.json' file still needs to be updated with the new version.

I hope you find my work worthy of being merged into your repo. It's my first open-source contribution :)

@MiguelRoldao
Copy link
Author

This feature came to me, as I am writing my master's thesis in latex and was finding it very annoying to have to enter math mode and to quote text manually :P

help/quoter.md Outdated

## Quoting modes

Depending on the type of the current file, Quoter can apply different quote and grouping symbols. For example, while in most languages backticks are usually used in pairs, as in `` `backticked` ``, to represent strings, infix operators, etc., in latex, to quote text, one would quote it with a backtick and an apostrophe, as ` ``quote'' `. Quoting modes allow to resolve these conflicting cases.
Copy link

Choose a reason for hiding this comment

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

This also applies to other lines but lines in text files are usually limited to 80 columns, so I think the lines have to be split.

help/quoter.md Outdated

`setlocal quoter.mode 'mode'`

Check the file "micro-quoter/modes.lua" in the plugin folder to see the differences between the different modes.
Copy link

Choose a reason for hiding this comment

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

Paths are usually inserted in backticks instead of double quotes.

Copy link

@niten94 niten94 left a comment

Choose a reason for hiding this comment

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

I'm sorry that I've accidentally started 2 different reviews with one comment. The commit message is long, but I think writing changes in summary and purpose in body can be done.

You can amend the commit to edit the commit message and changes done without adding another commit. If there is something you are not sure with, please feel free to ask.

help/quoter.md Outdated
Comment on lines 30 to 31
The mode of the current file can also be changed in the command-bar with:

Copy link

Choose a reason for hiding this comment

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

The line is not long so I think it is better if the empty line is removed.

quoter.lua Outdated

function init()
config.RegisterCommonOption("quoter", "enable", true)
config.RegisterCommonOption("quoter", "mode", "")
Copy link

Choose a reason for hiding this comment

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

I think it is fine if the default value is changed to "default" because it is the default mode used, and or modes["default"] at line 22 will not be needed.

@@ -1,13 +1,12 @@
VERSION = "1.0.2"
VERSION = "1.1.0"
Copy link

Choose a reason for hiding this comment

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

It is probably better to update repo.json in this pull request too if it will be changed in source code.

help/quoter.md Outdated
Comment on lines 5 to 6
Once installed, enable the plugin by setting `quoter.enable` to on:

Copy link

Choose a reason for hiding this comment

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

Same suggestion with removing empty line before `setlocal quoter.mode 'mode'`.

help/quoter.md Outdated

#### Extending Quoting Modes

New modes can be added by extending the `modes` table. If you have sophisticated quoting requisites that fall outside the implemented modes, feel free to extend the `modes` table and do a pull request to the fork at https://github.com/MiguelRoldao/micro-quoter.
Copy link

Choose a reason for hiding this comment

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

This pull request targets upstream so making a pull request at the upstream repository has to be written instead. Changes to features added in pull requests would usually be suggested in upstream issues or pull requests when the original pull request has been merged, and suggested in the pull request thread when not.

It would also be better to display URLs with a label. The URL is at the last part of the line, so I think it is fine if it exceeds 80 columns.

Suggested change
New modes can be added by extending the `modes` table. If you have sophisticated quoting requisites that fall outside the implemented modes, feel free to extend the `modes` table and do a pull request to the fork at https://github.com/MiguelRoldao/micro-quoter.
New modes can be added by extending the `modes` table. If you have sophisticated
quoting requisites that fall outside the implemented modes, feel free to extend
the `modes` table and do a pull request [here](https://github.com/sparques/micro-quoter).

@MiguelRoldao
Copy link
Author

MiguelRoldao commented Jan 25, 2025

First of all thank you @niten94 for your feedback! It helps me make better PRs in the future :)

I agree with all the recommend changes. I'm just having trouble with amending the commit message to a shorter version. From what I know, the only way to do it is by amending the commit message locally, and then force push it. Is this not bad practice? Should I do it either way?

@niten94
Copy link

niten94 commented Jan 25, 2025

I have only pushed commits on branches that I manage, but force pushing to a branch would only be a bad practice when there are other people pulling and working on it. Force pushing to branches only used to submit changes in GitHub is usually fine.

Quoting modes enable different quotePairs for different files/filetypes
based on user settings.
@MiguelRoldao
Copy link
Author

MiguelRoldao commented Jan 25, 2025

I think everything from my part is done. Now the only things left is to add a new tag and upload the zip. But I'm guessing that's not for me to do?

@niten94
Copy link

niten94 commented Jan 25, 2025

Sorry, I was away. Yes, there are parts I just noticed now that need to be changed but only the author can add a new tag.

Copy link

@niten94 niten94 left a comment

Choose a reason for hiding this comment

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

Sorry. I thought there might be a bug in micro so I took a while with adding one comment, but I think everything else is fine now.

Comment on lines +29 to +34
if not quotePairs then
micro.InfoBar():Error("Unknown quoter mode: \"" .. mode .. "\". Appliying mode \"default\".")
-- config.SetGlobalOption("quoter.mode", "default")
mode = "default"
quotePairs = modes["default"]
end
Copy link

Choose a reason for hiding this comment

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

There is a typo and I realized that it wouldn't probably be good if an error is always displayed, so I think these changes can be done:

Suggested change
if not quotePairs then
micro.InfoBar():Error("Unknown quoter mode: \"" .. mode .. "\". Appliying mode \"default\".")
-- config.SetGlobalOption("quoter.mode", "default")
mode = "default"
quotePairs = modes["default"]
end
if not quotePairs then
bp.Buf:SetOption("quoter.mode", "default")
quotePairs = modes["default"]
micro.InfoBar():Message("Unknown quoter mode: \"" .. mode .. "\". Mode is set to \"default\".")
end

Copy link

@niten94 niten94 Feb 2, 2025

Choose a reason for hiding this comment

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

Actually, using a different mode (default) is not expected so it's probably better to instead only display an error about changing it (like in settings.json then running reload), and returning false to not insert the character.

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.

2 participants