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

[CLOSED] Switch language / syntax mode of current document #5829

Open
core-ai-bot opened this issue Aug 30, 2021 · 30 comments
Open

[CLOSED] Switch language / syntax mode of current document #5829

core-ai-bot opened this issue Aug 30, 2021 · 30 comments

Comments

@core-ai-bot
Copy link
Member

Issue by peterflynn
Wednesday Jan 08, 2014 at 09:32 GMT
Originally opened as adobe/brackets#6409


This is an update of@JakeStoeffler's PR #4276. It's merged up with the latest master and addresses the main code review comments I made there. I also added some simple unit tests.

I still have two concerns about merging this immediately, though:

1) The dropdown UI doesn't look great. It uses a native <select>, which is just hard to style... and the way the statusbar is laid out makes it look misaligned in this case:
language picker

It would look nicer if we used an HTML popup like code hints, context menus, and the Recent Projects dropdown... but it would definitely add a bunch more work here. So maybe we can skate by with the current <select>-based UI for now. @larz0, any thoughts?

2) This foregrounds all the bugs in #2911 a lot more. I added a checklist of issues there. Are there any that are prominent enough to act as blockers here? (JS code hints seems like it might be the most glaring issue, but unfortunately that's one of the harder ones to fix too).


peterflynn included the following code: https://github.com/adobe/brackets/pull/6409/commits

@core-ai-bot
Copy link
Member Author

Comment by TomMalbran
Wednesday Jan 08, 2014 at 17:02 GMT


The algiment issue is caused by the padding around the actual select. The select doesn't have any border, but it still starts where the "LESS" text is. We could fix it by removing the padding around the select and placing it into the select.

@core-ai-bot
Copy link
Member Author

Comment by lkcampbell
Friday Jan 31, 2014 at 03:17 GMT


@peterflynn,@jasonsanjose, if you guys are in the process of figuring out the Sprint 37 pull requests to merge, this one gets my vote. This functionality would be very useful, especially if the document specific preference keep track of the language assigned to the document.

@core-ai-bot
Copy link
Member Author

Comment by njx
Tuesday Feb 04, 2014 at 00:50 GMT


I added this to Nominations on the Kanban board.

@core-ai-bot
Copy link
Member Author

Comment by busykai
Thursday Feb 27, 2014 at 18:51 GMT


@peterflynn, do you have time to merge the current master to this PR? I'll see how bad the issues from your checklist in #2911 look like... The first three seem to be quite important to address.

@core-ai-bot
Copy link
Member Author

Comment by busykai
Friday Feb 28, 2014 at 19:06 GMT


@peterflynn, merged master into your branch locally. let me know if you want me to push it to your branch.

bad alignment can be solved by adding left padding and the same amount of negative margin to the select. i have this fix in my local branch as well.

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Friday Feb 28, 2014 at 19:10 GMT


Sounds good -- thanks!

@core-ai-bot
Copy link
Member Author

Comment by TomMalbran
Friday Feb 28, 2014 at 22:44 GMT


Instead of using a select can we use something similar to the button dropdown used in the File Filters and CSS Quick Edit, once the File filters are merged?

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Friday Feb 28, 2014 at 23:02 GMT


@TomMalbran That's a great idea! Would probably make it easier to indicate the 'current' value too (e.g. placing a bullet or checkmark next to it as in menu-based picker UIs).

@core-ai-bot
Copy link
Member Author

Comment by TomMalbran
Friday Feb 28, 2014 at 23:05 GMT


The design is nicer too and won't have the same problems as the select has now. The checkmarks will look good too. We just need to wait until the Find Filters are merged to be able to use it.

@core-ai-bot
Copy link
Member Author

Comment by busykai
Saturday Mar 01, 2014 at 12:25 GMT


@TomMalbran great idea indeed! The native select looks out of the place, but I was struggling to find existing replacement. I'll fetch it once it lands.

@core-ai-bot
Copy link
Member Author

Comment by busykai
Friday Mar 07, 2014 at 00:56 GMT


OK, so #7015 has landed, I can take the widget from there now...

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Friday Mar 07, 2014 at 01:20 GMT


Cool, yay!

@core-ai-bot
Copy link
Member Author

Comment by busykai
Tuesday Mar 11, 2014 at 20:48 GMT


lang
This is how it looks like with the DropdownButton.

@core-ai-bot
Copy link
Member Author

Comment by larz0
Tuesday Mar 11, 2014 at 20:56 GMT


Looks good! Thanks@busykai

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Tuesday Mar 11, 2014 at 22:36 GMT


@busykai Might want to increase the allowable height. In the early usage of this in the file-filtering UI (0bbaf7a5) I used DropdownButton.dropdownExtraClasses to apply a class with max-height: none;. (Though since this list can get quite long, we probably want to pick some reasonable but not unlimited max-height -- sort of like the Quick Open dropdown does).

@core-ai-bot
Copy link
Member Author

Comment by busykai
Thursday Mar 13, 2014 at 15:33 GMT


@peterflynn, great idea to workaround scrolling! thanks!

@core-ai-bot
Copy link
Member Author

Comment by busykai
Thursday Mar 13, 2014 at 18:57 GMT


@jasonsanjose, ready for review when you have time.

@dangoor, if you're not too busy, please take a look at JavaScript code hints part. I did change the behavior for the active editor, but did not touch building scope -- it's a long way to the document.

@core-ai-bot
Copy link
Member Author

Comment by jasonsanjose
Thursday Mar 13, 2014 at 19:01 GMT


@busykai I'm actually reassigned currently.@dangoor can you assign at the next standup? Removing assignee for now.

@core-ai-bot
Copy link
Member Author

Comment by busykai
Thursday Mar 13, 2014 at 19:06 GMT


yeah, i was supposed to review this, but got "contaminated". sorry,@dangoor. :)

@core-ai-bot
Copy link
Member Author

Comment by dangoor
Monday Mar 17, 2014 at 13:15 GMT


@jasonsanjose OK, we'll reassign.

No worries@busykai, thanks for working on this!

@core-ai-bot
Copy link
Member Author

Comment by JeffryBooher
Friday Mar 21, 2014 at 04:57 GMT


This works fairly well but I think it needs an auto selection at the top of the list (followed by a separator) which, when selected, would just pass NULL to the setLanguageOverride function. I can see a use where I changed an audio file or something archaic to SQL or something else archaic and then later forgot what type the file actually is without a way to get it back.

Another thing that I think is missing is that it isn't remembered between instances of Brackets. Restarting and opening the same file should remember the setting. That seems like maybe something that could go on the backlog but I'll leave it up to you if you think it's easy to implement.

Also closing the file (removing it from the working files list) and reopening it even during the same session loses this setting.

@core-ai-bot
Copy link
Member Author

Comment by TomMalbran
Friday Mar 21, 2014 at 05:50 GMT


I read that@dangoor was thinking about a new language preference that could be applied to map extensions to languages using the paths scope, with: { "*.inc": { language: "html" } }. I guess that we could do something similar here but using the file path as the scope, or should we use an extension scope by default, but if it was set in more specific scope, then we change that one?

@core-ai-bot
Copy link
Member Author

Comment by JeffryBooher
Friday Mar 21, 2014 at 07:22 GMT


@TomMalbran I think it could be a simple project scoped preference which is remembered

@core-ai-bot
Copy link
Member Author

Comment by TomMalbran
Friday Mar 21, 2014 at 07:34 GMT


@JeffryBooher Since you are setting a new extension to a specific file, it has to be scoped to that file or to every file with the same extension. If I have a special .temp file and it is just an html file, I can change the language, but then it should only apply to the current file or to every file with the same extension. The preference could either go in the Global file or in the Project one.

@core-ai-bot
Copy link
Member Author

Comment by dangoor
Saturday Mar 22, 2014 at 02:12 GMT


There are a couple of questions to think about:

  1. Is what's offered by this PR better than nothing, which is what we have today? (Will people be so irritated that it doesn't save? Or will people be more irritated by the fact that today there's no way to change which language the editor is using?)
  2. Adding saving to this PR adds some UX questions and likely some tweaks to preferences infrastructure. Is that really required before landing this?

I'm certain that people will want to be able to save these mappings. But, I'm not certain they'd want this feature delayed in order to get that, and I'm also not certain that how we persist this is clear cut. What if there is no project settings file? What if the file that is being changed is not even in a project?

@core-ai-bot
Copy link
Member Author

Comment by JeffryBooher
Monday Mar 24, 2014 at 17:31 GMT


@dangoor I'm fine with spinning off bugs for the issues and enhancements. I was thinking if they were easy to implement then we wouldn't need to.

@core-ai-bot
Copy link
Member Author

Comment by dangoor
Monday Mar 24, 2014 at 20:00 GMT


@JeffryBooher Yeah, that makes sense. It just wasn't clear to me that you were thinking of those things as optional for this PR.

@core-ai-bot
Copy link
Member Author

Comment by JeffryBooher
Monday Mar 24, 2014 at 22:34 GMT


@peterflynn there's still the one nit that needs to be cleaned up before merging.

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Tuesday Mar 25, 2014 at 01:16 GMT


I agree with@dangoor as well -- I think we should land the thin slice of functionality now and circle back on prefs integration later.

My only question is whether we should do anything in the UI to indicate that it's a temporary change -- avoiding confusion or frustration when the language override isn't remembered the next time the file is opened. @larz0 /@njx do you think it's worth trying to do anything simple here for that?

In the meantime I can make the tweak@JeffryBooher mentioned in brackets.js...

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Tuesday Mar 25, 2014 at 01:29 GMT


Btw@RaymondLim - do you think we have enough testing on the JS Code Hints part of the code change? I know that code can be fragile sometimes...

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

No branches or pull requests

1 participant